[openstreetmap/openstreetmap-website] Use Bootstrap close button (PR #3631)
Andy Allan
notifications at github.com
Wed Aug 3 11:01:56 UTC 2022
Thanks @AntonKhorev ! I'm happy to merge this since it's much better for keyboard accessibility, and it also removes some of our custom CSS. A few points that could be addressed separately, if you are interested:
* As per the bootstrap reboot guidelines, we try to avoid using top margins:
> Avoid `margin-top`. Vertical margins can collapse, yielding unexpected results. More importantly though, a single direction of margin is a simpler mental model.
It's not directly relevant here, but the less we do it the less it'll be copied in other parts of the site. I usually consider using top-padding instead, where necessary.
* Many of the close buttons [were refactored](https://github.com/openstreetmap/openstreetmap-website/pull/2901) recently to use flexbox for layout, rather than floating, but there's still some (as found in this PR) where the icons are floating around. These could be refactored to follow the convention used in the sidebar.
* I saw you added the aria-label in some cases, which is great, but not others. We could add it everywhere, for consistency but also so that when other contributors copy+paste code then they are copying examples with the aria-labels.
* The use of buttons within the geo-link `<a>` is causing keyboard focus issues, since both the link and the button are individually focusable, and are in different places. Here's an image showing the `<a>` link focussed, pressing `tab` then focusses the cross.

--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/3631#issuecomment-1203797930
You are receiving this because you are subscribed to this thread.
Message ID: <openstreetmap/openstreetmap-website/pull/3631/c1203797930 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20220803/fd764e5b/attachment.htm>
More information about the rails-dev
mailing list