✨ (grapher) improve SVG output for manual manipulation #3621
Conversation
|
This stack of pull requests is managed by Graphite. Learn more about stacking. Join @sophiamersmann and the rest of your teammates on |
|
Quick links (staging server):
Login:
SVG tester:Number of differences (default views): 4530 (41995b) Edited: 2024-05-27 12:11:52 UTC |
sophiamersmann
force-pushed
the
improve-for-figma
branch
2 times, most recently
from
33bacb2
to
7f00352
Compare
sophiamersmann
changed the title
sophiamersmann
force-pushed
the
improve-for-figma
branch
from
7f00352
to
f3bbab7
Compare
| {labelTextWrap && | ||
| labelTextWrap.renderSVG( | ||
| -verticalAxis.rangeCenter - labelTextWrap.width / 2, | ||
| bounds.left, | ||
| { | ||
| id: makeIdForHumanConsumption("axis-label"), |
There was a problem hiding this comment.
In theory, according to W3C rules, an ID should be unique for the whole page.
It's not a big deal that we're breaking that rule here (I think, at least), but still wanted to flag it just to go sure.
In this case, we could change it to horizontal-axis-label and vertical-axis-label and be good.
There was a problem hiding this comment.
I have renamed axis-label, but since we often have multiple Graphers on one page, I expect most of these IDs to be present on a page multiple times. I think as long as we don't use them, we should be fine. (I hope makeIdForHumanConsumption makes it clear that we shouldn't)
packages/@ourworldindata/grapher/src/horizontalColorLegend/HorizontalColorLegends.tsx
Outdated
Show resolved
Hide resolved
sophiamersmann
force-pushed
the
improve-for-figma
branch
from
f3bbab7
to
190265b
Compare
sophiamersmann
force-pushed
the
improve-for-figma
branch
from
190265b
to
73277b4
Compare
sophiamersmann
force-pushed
the
improve-for-figma
branch
from
73277b4
to
c7eabbd
Compare
Resolves #3607
Adds labels to SVG groups and elements for more intuitive manual inspection and manipulation.
Figma displays an element's ID as its title. I researched if there is a way to customize this and show something else as an element's title, e.g. a
data-nameattribute or so, but I couldn't find anything. Since it's a bit odd to assign IDs to elements like that, I added amakeIdForHumanConsumptionfunction that communicates that these IDs are for human inspection only and are not used in code, thus can be safely deleted.I also made an effort to groups elements together, e.g. a tick's line and label is rendered into one group element.
PS: I also fixed this annoyance 👇🏻
Figma screenshot