Accessibility: Add focus styles #3
Conversation
The colors come from the Palette. The link components are our own, composing the focus styles and propagating other parameters. This is done because elm-ui does not set link styles with the global layoutWith parameter.
| Bear in mind that focus outlines should not be removed altogether, | ||
| because they are used by people who rely on keyboard and/or screen reader navigation. | ||
| They should also have a good color contrast with the background. | ||
|
|
There was a problem hiding this comment.
Part of me really wants to throw in a link or two about focus styles here. I've had good results with people respecting them after they read a bit on them. What do you think?
There was a problem hiding this comment.
Yeah, absolutely! As a rule of thumb, I'm a big fan of self-describing code rather than comments to explain what code is doing, and, when needed, comments to explain why something unexpected is being done in code. This seems like a good start 👍
|
Hello @fpapado, Thank you for taking the time to make this PR, that's awesome! One thing I notice is that if you tab through the links, we can get multiple links highlighted (you can see it live at the deploy preview):
I think this is related to this issue: mdgriffith/elm-ui#47 (comment) So it looks like we need to do a layoutWith and add a focus style? |
Yeah, so the markdown in this starter is currently being rendered with |
|
Ah, I thought the multi-focus issue was fixed. We already use layoutWith in this PR, but the workaround seems to only work for buttons, not links. The focus happens because of a general sibling combinator ( Anyway, we can wrap the links in single elements, to prevent that. I'll try it out in this PR. |
|
That's exactly what I was wondering for Markdown. I then found this line in the examples repo, with the custom rendering https://github.com/dillonkearns/elm-pages/blob/master/examples/docs/src/MarkdownRenderer.elm#L90. Two more questions with that in mind:
|
|
Hi @dillonkearns, is there anything else I could do with this PR? :) |
|
Hello @fpapado! The links are looking good in the deploy preview now. Oh, and by the way, to your question, styles can be added by doing an Regarding this change, it would be ideal if it wasn't necessary to use a helper function when adding links (as you said in your comment here). I'm okay with merging this for now, but it would be nice to revisit this when that bug gets fixed in elm-ui. Would you mind adjusting the |
|
Hey @fpapado, since mdgriffith/elm-ui#47 was fixed upstream it looks like this should be resolved now. Thank you! |
Hello @dillonkearns!
Here with the PR for focus styles, as promised :) (Closes #2 )
It is a small PR code-wise, but I think the conceptual parts are larger.
What I'm aiming for is to enable people to adjust the styles to their application, without nuking them altogether. To that end, I left a few comments in the code and tried to centralise them in
Palette. I went with the blue colour already present. Blue works quite well for contrast (part of why it's the browser default). I also went with box-shadows, because they follow borders, such as in rounded buttons.The
elm-uiAPIs for focus styles are a bit rough in my experience. ThelayoutWithoptions only apply to buttons, but not to links. I've had to do this same exercise at a work project, and there were always a couple of places to add focus styles. That's the main reason for containing them inPalette.About Markdown Links
There is one part that I'm a bit stuck (not a blocker necessarily).
In the starter, links in Markdown have the default browser link styles, and I do not understand how that's possible! (Running locally at http://localhost:3000/blog/hello).
Meanwhile, in the elm-pages demo site (at https://elm-pages.com/blog/introducing-elm-pages), the links inside Markdown do not have link styles at all.
This to me hints that the former are not under
elm-ui, but the latter somehow are? Could you point me to the part of the code that handles that? I think that's the final place where we can have the "good by default" behaviour, or to add comments etc.Aside about elm-ui
I'm adding a couple of thoughts about elm-ui for accessibility here.
I'm doing it mostly because this is a good example of a large project using it, and it can showcase the API decisions and implications for accessibility.
If you have a contact with Matthew, I'd love to bring them up with him. I have extremely limited time and energy, and the impression I have so far is that much of elm-ui is focused on perf/V2. I'll probably post these to the elm-ui tracker if they look good to you.
1) Focus options
In Windows High-Contrast Mode (WHCM), used approximately by 4% of folks on Windows, box-shadows are hidden. This is by design. In those cases, preserving an outline (even transparent) will mean that it is visible in WHCM. This is something that should change at the
elm-uilevel, to either allow outlines, or include a transparent one by default.2) Opt-in vs opt-out
Something we mentioned on Twitter, would be to have the default browser focus styles if not otherwise specified. I am skeptical of people opting-in to them, and removing them without prompting the developers to replace them is a bit scary.
Similarly, something that came up would be a focus style constructor for browser defaults, such as
Element.defaultFocusStyle.3)
layoutWithSomething I'm wondering is why
layoutWithdoes not apply focus to links. I would guess that they can always be overridden withElement.focusStyle. Is it an omission? Something that was considered? I'm not sure! I think it would make things easier for rendering links in Markdown, where specifying focus styles ad-hoc is hard/impossible.Wrapping up
Thank you for your time and willingness to integrate these!