You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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
feel free to slack me for more info/guidance
The text was updated successfully, but these errors were encountered:
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
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
h1
and then a list of blog posts on the page might haveh3
heading.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-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 designThen you can have a single
element and a single style.
detail
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
The text was updated successfully, but these errors were encountered: