Convert publications to local rendering #3058
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
4 tasks
andysellick
force-pushed
the
tracking-details
branch
5 times, most recently
from
730445c to
26cbc4f
Compare
KludgeKML
force-pushed
the
tracking-details
branch
from
2ddeea0 to
93a7811
Compare
- create a new partial for use and modification by publications - currently just a copy of the existing _attachments.html.erb
KludgeKML
force-pushed
the
tracking-details
branch
from
93a7811 to
6a1c119
Compare
- modify the new partial and the publication presenter to use a different bit of the content item for generating the attachments list - specifically, use `details->attachments` instead of `details->documents`, which is pre-rendered HTML - `details->attachments` contains only the data for the attachments, allowing us to render them individually in the new partial using frontend components, which will allow us to add tracking later
- Add override data to support publication test - Update tests to make more relevant
andysellick
force-pushed
the
tracking-details
branch
from
fa280ed to
2f5cdec
Compare
andysellick
changed the title
leenagupte
approved these changes
Apr 17, 2024
| @@ -11,7 +11,8 @@ def details | |||
| end | |||
|
|
|||
| def documents | |||
| content_item["details"]["documents"].to_a.join("") | |||
| docs = content_item["details"]["attachments"].select { |a| a["locale"] == locale } if content_item["details"]["attachments"] | |||
There was a problem hiding this comment.
Oh, this is clever, so you only display attachments with the same locale as the page. Nice!
| <%= render 'govuk_publishing_components/components/attachment', { | ||
| heading_level: 3, | ||
| attachment: details, | ||
| margin_bottom: 6 |
This was referenced Apr 17, 2024
This was referenced Jun 7, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What
Rework form (publication) pages to render attachments locally in the application, rather than using the pre-rendered chunk of markup from the content item.
Specifically, rework the list of documents on the forms pages to use the content item data in
details->attachmentsto render the required content, in place of the data indetails->documents, which is pre-rendered. This has involved the creation of a new partial for publication pages, as the existing partial is used by other page types.Example form pages:
Why
Relates to previous work in alphagov/govuk_publishing_components#3820
Reworked version of changes in #3048
We need to put tracking on the details components that can (sometimes) appear beneath the list of attachments, and that's very hard to do here on pre-rendered markup - normally we'd just pass some options to the component. This work will be carried out in a follow up PR.
Visual changes
details->attachmentsdoesn't include custom thumbnails for PDFs, as previously displayed. We looked at getting this included in the content item, but ultimately decided with consultation from leads that the default thumbnail for PDFs is acceptable. This is for several reasons:With the new data it looks like the attachments display the
Request an accessible formatoption more than they used to. This seems to be valid (it happens if you pass an email address to the component via thealternative_format_contact_emailoption) and allowing users this option more often seems sensible, but it raises the question why some PDFs used to have this option and some didn't.See example below. Note how before only some of these PDFs had the accessible format option, but now all of them have.
Trello card: https://trello.com/c/xhASj1rp/765-add-tracking-to-details-component