Jw grid font #1191
Jw grid font #1191
Conversation
src/components/YAxis.js
Outdated
| @@ -17,7 +17,7 @@ const YAxis = ({ tickCt, scale, width }) => { | |||
| return ( | |||
| <g key={i} transform={`translate(0, ${pos})`} className="tick"> | |||
| <line x2={width} y2="0" strokeOpacity={i === 0 ? '1' : '.25'} /> | |||
| <text fill="#000" x="-32" y="0" dy=".32em">{fmt(v)}</text> | |||
| <text className="fs-14" fill="#000" x="-32" y="0" dy=".32em">{fmt(v)}</text> | |||
There was a problem hiding this comment.
This is a totally fine approach, but we currently set the font size of the .axis elements in _viz.scss. I think we should either refactor out the parts of .axis that we already have classes for or just adjust the font-size in .axis.
To my mind, the tradeoff is that this style of css leads to a lot more stuff in the HTML attributes and so for SVG it might be nice to keep the styling in component style classes (like .axis) and for the rest of the HTML we can use the simple classes approach.
On the other hand, it makes a ton of sense to keep things as consistent as possible across the code base which is a compelling argument for refactoring out the aspects of .axis that we have elsewhere.
@jpwentz which approach do you prefer?
src/components/TrendChart.js
Outdated
| @@ -155,7 +155,7 @@ class TrendChart extends React.Component { | |||
| until={until} | |||
| /> | |||
| <div className="mb2 clearfix"> | |||
| <div className="sm-col mb1 sm-m0 fs-10 bold monospace black"> | |||
| <div className="sm-col mb1 sm-m0 fs-14 bold monospace black"> | |||
There was a problem hiding this comment.
I think 14px might be a bit big given the rest of the surrounding content's font sizes:
10px
12px
14px
I'd vote upping the font size to 12px for now.
BTW, @brendansudol great work on that alternating X axis label hiding! It looks just as good at 10px as 14px:
|
This has been addressed. |
|
👍 Nice |
Peer Review of Simple Font Changes