Datawrapper embed styling #12306
Merged
Datawrapper embed styling #12306
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
|
Size Change: +273 B (+0.03%) Total Size: 921 kB
ℹ️ View Unchanged
|
emdash-ie
force-pushed
the
datawrapper-embed-styling
branch
from
95656b9
to
0053000
Compare
emdash-ie
force-pushed
the
datawrapper-embed-styling
branch
from
0e55a5b
to
8256c53
Compare
|
I took some screenshots of a Datawrapper graphic in CODE with these changes, across the different roles and mobile vs. desktop views: Old screenshots hidden: click to expandInline roleDesktop
Mobile
Supporting roleDesktop
Mobile
Showcase roleDesktop
Mobile
Thumbnail roleDesktop
Mobile
Immersive roleDesktop
Mobile
|
|
I have added a bottom border in supporting, showcase, and immersive: Old screenshots hidden: click to expandSupporting roleMobile
Desktop
Showcase roleDesktop
Mobile
Immersive roleDesktop
Mobile
|
emdash-ie
force-pushed
the
datawrapper-embed-styling
branch
2 times, most recently
from
81b0a98
to
cd2f632
Compare
|
I made some changes to the spacing and borders after reviewing with the visuals team. Here are screenshots of the results, made by running this branch locally along with a local frontend: Inline roleDesktop
Mobile
Supporting roleDesktop
Mobile
Showcase roleDesktop
Mobile
Thumbnail roleDesktop
Mobile
Immersive roleDesktop
Mobile
|
|
I’ve also taken some screenshots from main in my local setup. These show that the weird margin behaviour for immersive on mobile predates my changes (and I think doesn’t display when testing in CODE, so something about the local setup): Inline roleDesktop
Mobile
Supporting roleDesktop
Mobile
Showcase roleDesktop
Mobile
Thumbnail roleDesktop
Mobile
Immersive roleDesktop
Mobile
|
This is an example of the kind of role-independent styling I would like to do on datawrapper graphics, by matching on the URL of the interactive element.
This is an example of the kind of role-dependent styling I would like to make for datawrapper graphics.
Relative links work well locally in text editors and on github, whereas an absolute link like this can be mildly frustrating when following from a text editor.
This 32px margin collapses with the existing 16px paragraph spacing. What about breakpoints? (Should I be using spacing constants? Probably)
This palette colour both matches the hex colour I had manually used, and seems like the most appropriate “border” colour listed in the file. Hopefully it’s a good choice: I must remember to ask in the PR. Also, looking at this has reminded me that I should check how my styling looks in dark mode!
For roles which move the graphics across the border on the side, we’d like to add a bottom border to avoid the odd reappearance of the side border after the graphic.
These margins were too small: the request from the visuals team is that they be 32px.
I’ve added a story with a Datawrapper embed for each role, to have an easy way to look at the styling. I’ve also tweaked the Wrapper used in the InteractiveBlockComponent stories to match that used in the Figure stories. Since the layouts are similar, I think this should be fine?
I had misunderstood some of the desired padding, and accidentally included the bottom border even at mobile breakpoints. Now the top and bottom margins are the same in the mobile breakpoint for supporting, inline, showcase, and immersive roles, for datawrapper graphics.
emdash-ie
force-pushed
the
datawrapper-embed-styling
branch
from
764ad2a
to
9b2cb88
Compare
github-actions
bot
removed
the
run_chromatic
To make it clearer which CSS is specific to Datawrapper embeds, this commit splits the Datawrapper-specific CSS out from the general Figure CSS, and relies on the ordering of the CSS for having the Datawrapper-specific CSS override the Figure CSS. This does mean that there’s a risk the Figure CSS will be edited in a way that breaks the Datawrapper CSS without the editor realising this. For example, the Datawrapper CSS uses breakpoints that should be the same as the ones the Figure CSS uses. Hopefully this risk is outweighed by the improvement to the clarity of the CSS.
DanielCliftonGuardian
approved these changes
Oct 24, 2024
Comment on lines
189
to
190
| thumbnail: css``, | ||
| halfWidth: css``, |
There was a problem hiding this comment.
Can we remove this ?
There was a problem hiding this comment.
I’ll take a look!
There was a problem hiding this comment.
Have removed and replaced with null
There’s no Datawrapper-specific css for thumbnail or halfwidth roles, so I had been returning an empty SerializedStyles value. However, it looks like the css prop can take null, so there’s no need for me to do that! This commit replaces thoses empty css`` values with null.
DanielCliftonGuardian
added
the
run_chromatic
github-actions
bot
removed
the
run_chromatic
github-actions
bot
removed
the
run_chromatic
Because we’re currently using Datawrapper’s “update old versions” setting, any updates to the Datawrapper graphic used in the stories cause new UI changes that need to be approved, which is annoying. To mitigate this, I’ve created a new test graphic which can be kept for just this purpose, and not updated unless we want there to be changes in the UI tests.
|
While I had this branch deployed to CODE, I checked whether Datawrapper’s automatic dark mode is currently working, and it doesn’t really seem to be: 
It might be that the dark mode colours are not set up quite right on the test team, but I’m a bit surprised by the white background: we may need to do some work on this to get dark mode working right. |
|
Interesting: disabling one 
|
|
Ah yes, it was deliberately kept white to handle interactives which don’t support dark mode: #10578 Hmmm, it would be nice if we could change this! Maybe just for datawrapper graphics, or maybe for everything. I don’t want to look at it in this PR though. |
github-actions
bot
removed
the
run_chromatic
|
Meant to write this comment on this PR (in between my last two) but accidentally put it on another one:
|
github-actions
bot
removed
the
run_chromatic
|
Seen on PROD (merged by @emdash-ie 9 minutes and 47 seconds ago) Please check your changes! |
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?
This PR tweaks the styling of
InteractiveBlockComponentswhen they’re used for datawrapper graphics, i.e. when the URL starts withhttps://interactive.guim.co.uk/datawrapper-test/embedorhttps://interactive.guim.co.uk/datawrapper/embed.Why?
These styling tweaks have been requested by the visuals team
Screenshots
I have added some before and after screenshots in coments on this PR, and here they are reproduced in a table: