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

Use components for consultation attachments #3181

Merged
merged 1 commit into from
May 15, 2024

Conversation

andysellick
Copy link
Contributor

@andysellick andysellick commented May 10, 2024

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

What

Output attachments in consultation pages using the data from the content item, rather than the pre-rendered markup from the content item, and apply GA4 tracking to those attachments that contain Details elements.

This follows on from the work to convert publications in the same manner: #3058

Attachments on consultation pages can occur in three places, so this is a bit more complex than publications, although some of the code and tests have been copied from that previous work.

Example pages to test with:

Note: an interesting side point on margin - there's some code for logic around a margin to appear beneath boxes that appear at the top of in-progress consultations ('We are analysing your feedback') so that spacing is only applied if there's stuff beneath it. Pretty sure I've preserved the logic but worth checking.

Why

This work is for two reasons:

  • to convert the rendering of attachments to using components so that tracking can be added to Details elements within attachments (for the 'Request an accessible format' element)
  • to generally move away from the use of pre-rendered markup passed through content items

Visual changes

Mostly should be no visual changes, however this change does remove custom thumbnails for PDFs (as previously removed for publications). This has been agreed and widely discussed around the previous work.

Trello card: https://trello.com/c/tcTN1jbu/28-update-rendering-of-govspeak-attachments

@govuk-ci govuk-ci temporarily deployed to government-frontend-pr-3181 May 10, 2024 10:51 Inactive
- output attachments in consultations using the data from the content item, rather than the pre-rendered markup from the content item
@andysellick andysellick force-pushed the consultations-use-components branch from ba55826 to e8c8d5d Compare May 10, 2024 10:54
@govuk-ci govuk-ci temporarily deployed to government-frontend-pr-3181 May 10, 2024 10:54 Inactive
@andysellick andysellick marked this pull request as ready for review May 10, 2024 11:01
@andysellick andysellick requested a review from KludgeKML May 10, 2024 11:01
Copy link
Contributor

@KludgeKML KludgeKML left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM (I wonder if we should extract the presenter stuff into a mixin if it's going to be largely the same in all the presenters, but perhaps we wait until they're all done and then refactor)

@andysellick
Copy link
Contributor Author

Thanks @KludgeKML yes definitely something to consider once all the formats have been converted. There's certainly a lot of duplication here now, particularly in the tests.

@andysellick andysellick merged commit 12ef5cc into main May 15, 2024
15 checks passed
@andysellick andysellick deleted the consultations-use-components branch May 15, 2024 08:26
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.

3 participants