Remove taxon-list app component from collections #3844
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
MartinJJones
force-pushed
the
remove-taxon-list-component
branch
from
2f2209b to
b2059e5
Compare
MartinJJones
force-pushed
the
remove-taxon-list-component
branch
6 times, most recently
from
de8930b to
35057dc
Compare
MartinJJones
force-pushed
the
remove-taxon-list-component
branch
from
35057dc to
9832a2a
Compare
MartinJJones
changed the title
MartinJJones
force-pushed
the
remove-taxon-list-component
branch
from
fe70206 to
7aa60ee
Compare
hannalaakso
approved these changes
Dec 16, 2024
There was a problem hiding this comment.
LGTM 🎉 Swapping these out turned out to be a bit more complicated than we initially thought and this solution resolves it all quite cleanly without the need for adding extra options to the gem which is nice. I've compared the generated HTML of the relevant templates before and after and it all looks good. The extensive commit messages were also super helpful when reviewing the changes, thanks for doing that 🙏
Remove all files relating to the taxon-list component the taxon-list component as it is no longer required. The component was originally created for an AB Test on https://www.gov.uk/education/funding-and-finance-for-students#/education/student-grants-bursaries-scholarships This page is now using the document-list component from the govuk_publishing_components gem
Update data structure returned from `taxon_list_params` method Updated the structure to work with the document-list gem component, setting `full_size_description: true` ensures the font-size is set to 19px for the description when present - https://components.publishing.service.gov.uk/component-guide/document_list/with_full_size_description_text
- Use the `document-list` component from the gem instead of the `taxon-list` app component in the `tagged_content_list.html.erb` and `accordion.html.erb` view templates - By using the document-list component, the list will change from an ordered list to an unordered list, this feels like a slight improvement as the content presented does not appear to be in order of sequence/importance - `heading_level` variable removed from the `tagged_content_list.html.erb` template, the document-list component does not support headings and is not required here This approach is also consistent with other places the document-list component is used, for example: - [Education, training and skills - GOV.UK](https://www.gov.uk/education) (collections) - [Find contact details for services - GOV.UK](https://www.gov.uk/contact) (feedback) - [Travel or do business in Europe: Brexit guidance](https://www.gov.uk/government/collections/brexit-guidance) (government-frontend) - set `disable_ga4: true` to disable GA4 tracking - in the accordion.html.erb this prevents duplicate events from firing as this is already built into the accordion component - GA4 was not enabled previously in the tagged_content_list.html.erb
Replace the `taxon-list` component with the `list` component from the govuk_publishing_components gem. `list_type: "number"` has been used to ensure an ordered list is used to match the current HTML structure
Updated the tests to check that the "title", "link" and "description" for each search result/tagged content is included on the page
MartinJJones
force-pushed
the
remove-taxon-list-component
branch
from
7aa60ee to
da64b05
Compare
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
Why
The taxon-list component is no longer required
The taxon-list component was originally created for an AB Test on https://www.gov.uk/education/funding-and-finance-for-students#/education/student-grants-bursaries-scholarships. This page is now using the document-list component from the govuk_publishing_components gem
Accessibility improvement
Since the document-list component doesn’t support headings for lists of links, we’ve decided not to try and add this option into the component. This is because wrapping links in headings is not a suitable option for loads of GOV.UK pages since:
<p>tag) after it. Having a heading or a mix of headings and plain links doesn’t seem to be a good option here as it could be confusing for usersVisual changes
Spacing is increased slightly for list items when using the document-list component
https://collections-pr-3844.herokuapp.com/world/afghanistan (accordion.html.erb)
https://collections-pr-3844.herokuapp.com/world/living-in-afghanistan (tagged_content_list.html.erb)
https://collections-pr-3844.herokuapp.com/government/ministers/secretary-of-state-for-foreign-commonwealth-and-development-affairs (past_role_holders.html.erb) - No visual changes
Trello card