Change ID in image galleries to avoid duplicates and fix exhibitions test #11446
+72
−23
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
rcantin-w
added
epic: accessibility
|
Size Change: -41 B (0%) Total Size: 1 MB
ℹ️ View Unchanged
|
rcantin-w
changed the title
| .sort((a, b) => { | ||
| // Makes TS happy but this condition should never not work | ||
| // we always have an end date, even for permanent exhibitions | ||
| // e.g. Being Human ends in 2090 |
There was a problem hiding this comment.
"Being Human ends in 2090". I'll be happy if I've made it that far
| if (a.end && b.end) { | ||
| return a.end > b.end ? 1 : -1; | ||
| } | ||
| return 1; |
There was a problem hiding this comment.
should this not be return 0 (if there isn't a difference)?
There was a problem hiding this comment.
As discussed we're going to make so if there's no date it'll be considered to be year 2090
There was a problem hiding this comment.
| if (a.end && b.end) { | ||
| return a.end > b.end ? 1 : -1; | ||
| } | ||
| return 1; |
davidpmccormick
approved these changes
Dec 17, 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 does this change?
The way we split pages into various slice zones now (to allow for cookie tables or recommended stories), the
idlogic for image galleries now will result in duplicates (see failed pa11y test here). This logic simplifies it.I also had to amend the exhibitions test that used to fail every time an exhibition was added (see fail here). We changed it in the past so we wouldn't have to add them in every time by slicing the first 6 results only, but we sorted them by ID, and in this case, the new exhibition's ID rated higher than one of the old ones. So I changed the logic to use the exhibition's end date instead.
How to test
Duplicate IDs: Run locally: http://localhost:3000/stories/the-birth-of-britain-s-national-health-service
Exhibition test: Do they pass here?
How can we measure success?
No more pa11y flags/duplicate IDs
Have we considered potential risks?
N/A