Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Show first-introduced versions on roadmap #283

Merged
merged 4 commits into from
Sep 24, 2022

Conversation

andylizi
Copy link
Contributor

@andylizi andylizi commented Aug 23, 2022

This is a tentative implementation of the long-awaited feature proposed in #187.

https://webassembly.org/roadmap/

feature-table

The initial data set is from this comment.

Closes #187.

@juj
Copy link
Contributor

juj commented Aug 25, 2022

This looks absolutely fantastic!

@RReverser
Copy link
Member

This does look amazing! I can't do a proper review at the moment, could you ping me in a 3-4 days in case I forget? (others are welcome to review meanwhile, too)

@andylizi
Copy link
Contributor Author

I can't do a proper review at the moment, could you ping me in a 3-4 days in case I forget?

@RReverser ping~

@RReverser
Copy link
Member

Thanks, I built and played with this locally, and overall I like it a lot. Two notes:

  1. In previous version table cells with notes were focusable, which provided at least some primitive level of accessibility for keyboard users (although not a great one). Now that's replaced by [a] letters which also help to relate cells to footnotes at least for keyboard users, but perhaps it's not hard adding keyboard navigation alongside your mouseenter/mouseleave handlers so that visually impaired users could hear the level of support + notes for engine x feature too?
  2. While it's possible to figure out what JS does by interacting with page and setting breakpoints, JS code alone is becoming rather hard to follow and guess what parts of it are responsible for. Could you add some comments to toAlphabet function and the various parts of footnote logic to make future maintenance easier?

@andylizi
Copy link
Contributor Author

andylizi commented Aug 29, 2022

Comments should be a lot clearer now. Accessibility is definitely an issue, but I'm somewhat unsure of how to go about it, mostly because of my lack of knowledge in this area. It's easy to make the cells focusable (tabindex=0), but it is advised that screen readers won't read title anyway and they shouldn't be used for accessibility……? Would something like aria-describedby work in this case?

Initially I've considered showing a custom tooltip on hover/focus, like the references in Wikipedia and caniuse.com. But that would involve third-party libraries and generally be a great pain to implement.

I tried highlighting the footnotes on focus, but the blur/focusout events doesn't seems to trigger properly for some reason……

Anyway, it's midnight for me now. I will do more research on this tomorrow.

@andylizi
Copy link
Contributor Author

andylizi commented Sep 1, 2022

Done! I've implemented the custom tooltips and tested the accessibility with a screen reader. There should be no problem regarding that now.

By the way, I used a JS feature from ES2020, which is supported by all browsers released after 2018 or 2019. Is that acceptable for this website or should I add a fallback for it?

@RReverser
Copy link
Member

Awesome, thanks!

By the way, I used a JS feature from ES2020, which is supported by all browsers released after 2018 or 2019. Is that acceptable for this website or should I add a fallback for it?

Perfectly acceptable, I don't think we should worry about older browsers on a developer-centric website.

roadmap.js Outdated Show resolved Hide resolved
roadmap.js Outdated Show resolved Hide resolved
@EwoutH
Copy link

EwoutH commented Sep 22, 2022

This looks amazing! What's needed to merge this PR?

@RReverser
Copy link
Member

This looks amazing! What's needed to merge this PR?

Me having a better memory 😅

I'll try it one more time locally now and hopefully merge after that.

roadmap.js Outdated Show resolved Hide resolved
roadmap.md Show resolved Hide resolved
@RReverser
Copy link
Member

Left request for two small changes, the rest looks great!

@RReverser
Copy link
Member

Ok I think we can always iterate on further a11y improvements in separate PRs, this already looks much better than what we had. Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Please change roadmap checkmark chart to caniuse style "first introduced in"
4 participants