Implement interactive URL darkmode fix for additional path groups - #13756
Merged
Merged
Conversation
|
Hello 👋! When you're ready to run Chromatic, please apply the You will need to reapply the label each time you want to run Chromatic. |
|
Size Change: +34 B (0%) Total Size: 959 kB ℹ️ View Unchanged
|
simonbyford
reviewed
Apr 9, 2025
| url && url.includes('interactive.guim.co.uk/uploader/embed/') | ||
| url && | ||
| (url.includes('interactive.guim.co.uk/uploader/embed/') || | ||
| url.includes('superyacht') || |
Contributor
There was a problem hiding this comment.
Could this be a more suitable (i.e. less general) marker?
Suggested change
| url.includes('superyacht') || | |
| url.includes('interactive.guim.co.uk/embed/superyacht') || |
We'd just have to be sure all superyacht URLs have this form
Contributor
Author
There was a problem hiding this comment.
In the last audit test I ran I managed to catch 118 superyacht URLs, which is the same number you get when you search that sheet for interactive.guim.co.uk/embed/superyacht - which gives me some confidence that the tool has been set up to always dump its output into the same S3 bucket. https://docs.google.com/spreadsheets/d/1DF-6cBa5xQCH_1tKgc7x5F5eXakQYKz00IbMjywwjO0/edit?gid=1748685808#gid=1748685808
Contributor
There was a problem hiding this comment.
That sounds promising. Shall we make the change?
Contributor
There was a problem hiding this comment.
Oh I see, you're making the opposite argument.
Sure, I'm fine to leave it as superyacht 👍
simonbyford
approved these changes
Apr 9, 2025
simonbyford
left a comment
simonbyford
left a comment
Contributor
There was a problem hiding this comment.
Tested on CODE and everything looks good ✅
A created an article containing 3 embeds (one superyacht, one choropleth_map_maker, one unaffected) and the URLs were as expected:
Contributor
Author
TestingBefore
After
|
|
Seen on PROD (merged by @KaliedaRik 10 minutes and 16 seconds ago) Please check your changes! |
groakland
added
maintenance
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
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?
This work extends the work to add a
darkmode=falsesearch query to the end of eligible interactive embed URLs, as detailed in PR #13647Why?
Following the merging of the previous PR into production, Australian colleagues have requested we add similar functionality for two types of interactives they create:
superyacht(for chart embeds)choropleth_map_makerScreenshots
See testing results comment below
Testing
Testing in the CODE environment:
superyachtorchoropleth_map_makershould now include the appended search query string - either?darkmode=falseor&darkmode=falseSuitable existing interactives URLs for this test include:
Superyacht
choropleth_map_maker
Unaffected URLs