Convert admonition blockquotes to sections for screen reader users #1938
Conversation
Some screen readers will announce that an element is a blockquote. It could be potentially surprising to hear something announced as a blockquote despite the inner contents not being a quote.
lib/ex_doc/markdown/earmark.ex
Outdated
| |> String.split(" ") | ||
| |> Enum.filter(&(&1 in @admonition_classes)) | ||
| |> Enum.join(" ") | ||
| end) |
There was a problem hiding this comment.
I tried to keep everything self-contained instead of adding helper functions, so that's why this is a little messy. The "coercion" to "" is there since the call to Enum.join/2 will return an empty string for an empty list.
|
Might not need this javascript anymore. |
|
Would it make sense to turn this into a |
|
I don't see why not! I'll push some changes. |
voughtdq
changed the title
|
/cc @DavidOliver, who, iirc, worked on this feature. /cc @wojtekmach, who, iirc, worked on fixup. @voughtdq can you please take a look at tabset.css and print.css? They also reference blockquotes and I think they should be addressed as well. There may be others. |
That looks like the case as well. We should give it a try. Thank you! |
|
I think we're good to go. @DavidOliver can you take a look? |
|
@voughtdq, so far I've only looked through the latest CSS changes - I haven't tried out your branch yet - but I've only found one thing remaining, which I've commented on directly. I would usually tend not to rely on Thanks. |
|
(Saw your other comment about the selector I missed, pushed those changes - sorry about that). I agree that using The problem I'm running into with > ### Warning {: .warning}
> Don't feed your dog too much peanut butter.
>
> #### What about my cats?
> They don't really like peanut butter.will end up having the second In this case, I think it's pragmatic to use That said, I never have strong opinions on these things, so I'm open to alternatives. The alternative I can think of is adding an additional class (e.g. |
|
@voughtdq, I see. Thanks! |
|
@voughtdq given that we control the markup, we can adjust the markup by adding a .header or title to the h3/h4. As you prefer. Whatever leads to the best combination of code + CSS. However, I am remembering that we explicitly chose to not go down this path. We didn't want to change the generated Markdown because that would further couple us to the underlying markdown engine and we didn't want to have a subset/superset of Markdown. So it was very important that our rules were expressed with only CSS. Perhaps it is fine to deviate at this point, but I wanted to write down why we didn't go with this approach early on. |
This is a common practice with other markdown/documentation generators.
|
I took some time to consult how other markdown/documentation implementations do it. There is a consensus of setting a class for the admonition title, but not on what elements/classes to use. I see a lot of implementations that use There is a lot of fragmentation in markdown admonition syntax and then further fragmentation in what HTML is generated for it. I anticipate that any new engines added, markdown or otherwise, will require transformation of the admonition markup simply because there's no true consensus or standard. |
lib/ex_doc/markdown/earmark.ex
Outdated
| blockquote_meta} | ||
| ) | ||
| when tag in ["h3", "h4"] do | ||
| {h_classes, h_attrs} = Enum.split_with(h_attrs, &match?({"class", _}, &1)) |
There was a problem hiding this comment.
You can also use List.keytake and similar. :) Similar below.
There was a problem hiding this comment.
Ok, I'm glad to hear that 😅
I think I can make it look better with keytake/keystore
There was a problem hiding this comment.
If you take, you probably don't need keystore, you can just prepend it: [{"class", val} | attrs]
|
@voughtdq I dropped one minor nitpick. Also, can you please go ahead and delete the JavaScript function you linked to? https://github.com/elixir-lang/ex_doc/blob/v0.34.2/assets/js/content.js#L37-L45 Then I will merge it! Thank you! |
|
Working on that when I wake up. I have a clearer implementation with List.keyfind/List.keystore, but it also means different ordering of attrs in some cases. Is that ok? I will go back to keytake/append if not ok. |
|
Yes, it is ok! |
keyfind/keystore didn't really improve things that much, so using keytake/prepend as originally suggested
|
💚 💙 💜 💛 ❤️ |
Some screen readers will announce that an element is a blockquote. It could be potentially surprising to hear something announced as a blockquote despite the inner contents not being a quote.
References:
Closes #1939