Closed SachaG closed 3 years ago
A short list of what's not complete/up to date as of right now (so you can ignore it/come back to it later):
Everything else should be good to go!
😍 Looks so good, congratulations! Can tell you & Raphaël have been polishing this for a while.
Some things I noticed that may need revisiting:
/resources/sites-courses
, "Wes Bos Courses (GSSGrid.io, Flexbox.io, etc.)
")Woo! So excited for all this hard work to be shown to world. Congrats and thanks again for all the value this will provide to the community.
I found a couple areas of improvement for accessibility:
#ec2f95
to #f055a9
, or increasing the HSL lightness from 55% to 64% would ensure a minimum 4.5:1 contrast ratio.<h1 class="Sidebar__Logo__Wrapper Logo__Wrapper">
contains multiple <a>
tags and some block level elements, such as <button>
. Additionally, there is no actual heading text within the <h1>
, as it's being used as a layout container. I would recommend having a visually-hidden <h1>State of CSS 2019</h1>
so there is a clear, concise heading for the entire page, and making .Sidebar__Logo__Wrapper.Logo__Wrapper
a <div>
for styling.
.screen-reader-only {
padding: 0;
width: 1px;
height: 1px;
position: absolute;
clip: rect(1px, 1px, 1px, 1px);
clip-path: inset(0px 0px 99.9% 99.9%);
overflow: hidden;
border: 0;
}
role="presentation"
or aria-hidden="true"
to remove them from the Accessibility Object Model (AOM). This prevents screen readers from announcing irrelevant information. I see that this is being done on the social sharing button icons, which is great. However, it looks like these links have an empty aria-label
(maybe a data-binding issue?). I would instead recommend including visually-hidden text inside of the links and ensuring that this text is rendered to the DOM.<div class="Sidebar Sidebar--hidden">
would benefit from being a <nav>
element.<div class="pagelayout__content">
or <div class="pagelayout__main">
would benefit from being a <main>
element (the markup/layout structure is unique so this is a tough call).<div class="Logo__Container Logo--l">
can benefit from aria-hidden="true"
, since it is decorative.<h3>
headings, but no <h2>
exists on the page. These headings can use <h2>
instead.That's everything I noticed on an initial pass. The site overall looks great and I can't wait to go through all the amazing content! Also, huge props for creating dedicated keyboard focus styles 🙌
@loklaan thanks, I fixed most of these!
@davidluhr wow, thanks so much for the detailed review! A few thoughts/questions:
.screen-reader-only
class you suggested? This way the dropdown would always be accessible even in its hidden state (I'd rather not use a third-party thing if I can avoid it). This is great! I spotted a layout bug (ha!):
PS I wonder if that chart is a bit misleading as it is a ranking of the frameworks but it looks like a graph so implies something quantitive. At the very least it would be good to get the raw number when you hover -- the chart tells me for instance than emotion has a better satisfaction rating than styled-components, but is it close? What are the actual numbers?
@tmeasday, thank you for your feedback!
We're planning to add the actual values to this chart as it can be misleading because sometime the gap is small between 2 technologies.
@SachaG You're most welcome. I'm very excited to see this come together!
<div class="LanguageSwitcher__Toggle">
a <button>
so it can receive keyboard focus. On click/keyboard interaction, the dropdown should open, and focus should be moved to the first focusable item in the dropdown. Using the Tab
key should cycle/trap focus within the dropdown menu options, and using Esc
or clicking outside the menu would close the menu and return focus back to .LanguageSwitcher__Toggle
. This functionality would ensure that all users can change the language as needed. What are your thoughts?Thanks, I'll try that. Btw check your Twitter DMs :)
He guys, Great job on this repo, is really inspiring. Just a quick question, why use scss + styled components instead of creating containment and line breaking in styled components? Again, thanks for sharing this repo I am learning loads from it <3
@AntonioAsr basically we improve things a little bit every time we do a new survey, for example check https://github.com/StateOfJS/StateOfJS-2019/ where we use Styled Components more.
You can preview the State of CSS results here:
Feedback welcome! By the way, although this link is "public" please avoid sharing it on social media. We don't want to spoil the launch :)