Some repetition on styling and incorrect markup.
Important to change due to "feature creep" & correct semantic tags are needed for accessibility and correct crawling
example: the main page title might be h1 and then a list of blog posts on the page might have h3 heading.
paragraph , <p> is just a paragraph. there's no <p2> because it has a single level of meaning
https://github.com/overture-stack/website/blob/develop/src/components/Typography/styles.scss#L76
The style for p2 only change for breakpoints - all other attributes are the same. This is a sign that there's likely a change could be made to have one element, one style - often this is a very very slight design variable isn't in line with the rest of the design. As seems to be the case here, line-height is the only different attribute.
Once code starts branching into x1, x2, x3 etc for minor changes and the codebase gets established, things become messy, and then no one wants to fix it - good to nip it in the bud.
possible impl
Firstly double check that this design is correct.
Ensure line-height has to change in this situation.
If yes, I would make an extra class to apply just the line-height style, because it's likely to occur again in the design
-so it matches with the correct web you're using
similar to <p> , L1 isn't a thing, only <li> and in this case the element you're wrapping is <ul> not <li>
So reading <Li><li>item</li></Li> is very confusing
Detailed Description
Some repetition on styling and incorrect markup. Important to change due to "feature creep" & correct semantic tags are needed for accessibility and correct crawling
1) P2 "doesn't make sense" https://github.com/overture-stack/website/blob/develop/src/components/Typography/index.js#L32 The reason is
<h1>,<h2>,<h3>and so forth are actual html elements. They have semantic meaning attached to them.example: the main page title might be
h1and then a list of blog posts on the page might haveh3heading. paragraph ,<p>is just a paragraph. there's no<p2>because it has a single level of meaninghttps://github.com/overture-stack/website/blob/develop/src/components/Typography/styles.scss#L76 The style for p2 only change for breakpoints - all other attributes are the same. This is a sign that there's likely a change could be made to have one element, one style - often this is a very very slight design variable isn't in line with the rest of the design. As seems to be the case here,
line-heightis the only different attribute.Once code starts branching into x1, x2, x3 etc for minor changes and the codebase gets established, things become messy, and then no one wants to fix it - good to nip it in the bud.
possible impl
Firstly double check that this design is correct. Ensure
line-heighthas to change in this situation. If yes, I would make an extra class to apply just theline-heightstyle, because it's likely to occur again in the designThen you can have a single
element and a single style.
detail
2) minor naming. https://github.com/overture-stack/website/blob/develop/src/components/Typography/index.js#L32 Please rename this component to
-so it matches with the correct web you're using similar to
<p>, L1 isn't a thing, only<li>and in this case the element you're wrapping is<ul>not<li>So reading<Li><li>item</li></Li>is very confusingfeel free to slack me for more info/guidance