Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Move specialist document pages #4600

Draft
wants to merge 19 commits into
base: main
Choose a base branch
from

Conversation

leenagupte
Copy link
Contributor

⚠️ This repo is Continuously Deployed: make sure you follow the guidance ⚠️

What

Why

Trello card

How

Screenshots?

@govuk-ci govuk-ci temporarily deployed to govuk-frontend-app-pr-4600 January 17, 2025 17:28 Inactive
Changes the view to call the title component directly, rather than via
a presenter method.
The list of headers is used to generate the contents list if there are
more than 3 headers in the list.

Confusingly this list of headers is generated in multiple places.

[specialist-publisher] uses the govspeak gem to parse the body content
and look for `h2` and `h3` tags and generate the `headers` list in
`details` from them. If there are no `h2` or `h3` tags the entire
`headers` element is removed from `details.

government-frontend deals with `headers` in two different ways. It uses
the `headers` element in details to [create the contents list], but then
somewhat replicates the code from the govspeak gem by [parsing the `h2`
tags] in the body content to determine whether or not so show the
contents list.

Those it's not being used to render the contents list in
government-frontend the `headers` element does have a `level` attribute
that indicates whether the header is a `h2` or `h3`. Therefore it should
not be possible to greatly simplify the code from its current form in
government-frontend.

The first step is to just extract the headers list in the
SpecialistPublisher model. The code to determine whether or not to
display the contents list will be added to a presenter in a later
commit.

An example from content schemas is being used to initialise the
SpecialistDocument model object, however  the ["details"]["headers"]
have been copied from the example into the test as unlike other model
tests the values in the array are being modified and the test is easier
to follow if you can see exactly what's changed.

Audit trail:
specialist_document_presenter: https://github.com/alphagov/government-frontend/blob/c6eead39b29192e692c802c9b5e8bc234e16692c/app/presenters/specialist_document_presenter.rb#L86-L100

[specialist-publisher](https://github.com/alphagov/specialist-publisher/blob/efadf3c4df33d045c0067e095f958d4b12638f29/app/presenters/document_presenter.rb#L50)
[create the contents list]: https://github.com/alphagov/government-frontend/blob/c6eead39b29192e692c802c9b5e8bc234e16692c/app/presenters/specialist_document_presenter.rb#L86-L100
[parsing the `h2` tags]: https://github.com/alphagov/government-frontend/blob/main/app/presenters/content_item/contents_list.rb#L43
This adds a `show_contents_list?` method. The method needs to know if
there are any level two headers. Rather than parsing the html in the
body attribute, the `level` is just being preserved in the `headers`
list in the model.

There is a lot of code in the original government-frontend code relating
to images, and the character count and the number of table rows. This
seems to be more related to document collections, rather specialist
documents, so for now it has been omitted and rather than creating a
shared presenter, a specialist specific one has been created to minimise
the code.

Audit trail:

contents_list_presenter.rb: https://github.com/alphagov/government-frontend/blob/main/app/presenters/content_item/contents_list.rb#L21
This updates the view to use the new `show_contents_list?` method from
SpecialistDocumentPresenter.
@leenagupte leenagupte force-pushed the move-specialist-document-pages branch from b259d63 to 35c7372 Compare January 17, 2025 17:29
@govuk-ci govuk-ci temporarily deployed to govuk-frontend-app-pr-4600 January 17, 2025 17:29 Inactive
@leenagupte leenagupte force-pushed the move-specialist-document-pages branch from 35c7372 to 6f7fd4c Compare January 17, 2025 17:31
@govuk-ci govuk-ci temporarily deployed to govuk-frontend-app-pr-4600 January 17, 2025 17:32 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants