Use mark instead of span #75
Conversation
RoyEJohnson
force-pushed
the
use-mark-instead-of-span
branch
from
757c147 to
6bbd6bd
Compare
RoyEJohnson
force-pushed
the
use-mark-instead-of-span
branch
from
eb8e345 to
cfc0ab6
Compare
RoyEJohnson
force-pushed
the
use-mark-instead-of-span
branch
from
cfc0ab6 to
7af5df5
Compare
|
@RoyEJohnson Could you please get this deployed somewhere. I believe highlighter does not get a direct deployment link like rex-web. |
RoyEJohnson
force-pushed
the
use-mark-instead-of-span
branch
from
1af3ac7 to
7af5df5
Compare
|
Testing link: openstax/rex-web#2158 |
RoyEJohnson
force-pushed
the
use-mark-instead-of-span
branch
5 times, most recently
from
5a33dee to
d7a26d9
Compare
Default tabbable to true Update snapshots
RoyEJohnson
force-pushed
the
use-mark-instead-of-span
branch
from
d7a26d9 to
4296d75
Compare
RoyEJohnson
force-pushed
the
use-mark-instead-of-span
branch
from
f87a4fc to
4f61498
Compare
RoyEJohnson
force-pushed
the
use-mark-instead-of-span
branch
from
8b94268 to
7869c30
Compare
It seems that some offsets needed to be recalculated due to removing screenreader nodes.
|
@TomWoodward The last four commits take care of issues created by removing the actual screenreader nodes and using pseudoelements instead. |
src/Highlight.ts
Outdated
| public addFocusedStyles(): Highlight { | ||
| this.elements.forEach((el: HTMLElement) => el.classList.add(FOCUS_CSS)); | ||
| this.elements.forEach((el: HTMLElement) => { | ||
| el.setAttribute('aria-current', 'true'); |
There was a problem hiding this comment.
do you have any literature about aria-current ? i don't know much about it but the mdn page makes it seem like it might be meant for a current element in a discrete list, and i'm concerned that our mark elements are spread over the whole content and not in a group like that
There was a problem hiding this comment.
I can take aria-current out. (It certainly shouldn't bearia-selected.)
The screenreader should distinguish start-selected-highlight vs start-highlight and maybe that's all it needs.
There was a problem hiding this comment.
I have updated the Highlighter branch to remove aria-current and restore the old focus class (for CSS selection purposes). A Rex branch for testing it is here
There was a problem hiding this comment.
is it not updated in this PR?
There was a problem hiding this comment.
Whoops, forgot to push. This PR is updated now.
RoyEJohnson
force-pushed
the
use-mark-instead-of-span
branch
from
72beac0 to
6384d5f
Compare
|
I think the changes in this branch went too far. I have another branch which I hope will be less trouble. |
For: https://github.com/openstax/unified/issues/2647
For: https://openstax.atlassian.net/browse/DISCO-111
For: https://github.com/openstax/unified/issues/2718
For: https://openstax.atlassian.net/browse/DISCO-112
DISCO-227