accessibility improvements for quick format dialog #3464
Conversation
|
@adomasven, I believe you would be the best person to look over this? |
|
Testing with VoiceOver, things kind of break and not work as expected if you shift-tab to the Zotero button, and then try to tab back, especially if the reference panel is open and there are multiple citation bubbles. Like if you try to delete a bubble it deletes characters, the cursor gets tuck and unable to jump between bubbles, etc. If I open a menu either on the Zotero button, or in the popup panel for locator selection, then VoiceOver gets stuck. Not sure how addressable that is in general. We should probably have some aria property which make it say that you can search for items to cite when the cursor is in the search box.
Yes, you cannot have span bubbles in input or textarea fields. I've been wanting to rewrite the citation dialog ever since I joined Zotero and I have plans for that, but there are always other things that are higher priority. The bubble keyboard navigation is janky and the jankiness is probably unfixable, and then it's doubly so with accessibility tools. I don't know how far we can get with simple aria hints here without fixing the finickiness of the dialog itself, and I've tried to make it more reliable in the past, but it requires a ton of experimentation with non-obvious stuff. |
|
On general citation bubble jankiness, the single worst aspect is the transition to opening the dropdown to add a locator. This is unintuitive, and undiscoverable, and often doesn’t seem to catch properly. And you 100% always want to do this via keyboard, accessibility issues aside. |
|
(OT, but for the undiscoverable part, we should just add a dropmarker to the bubbles, at least on hover.) |
|
@adomasven, thank you - I will tweak things with VoiceOver to try to mitigate the jankiness you pointed out. I did have a more general suggestion that should make it easier to finish up the work with screen readers and also help with jankiness overall. I think it would also solve the 2 existing glitches I pointed out. Most challenges I ran into come from the fact that the cursor combined with the focus on bubbles can confuse the screenreaders that are quite opinionated about how focus is handled. VoiceOver, as you noticed, gets thrown off when we jump between focusing on bubbles to focusing on input for typing and back. Other issues come from tricky handling of typed characters that were not turned into a bubble yet. For example, you can type characters in multiple places for what will be one new bubble, or when there's an out-of-bubble character, the cursor can be moved inside of another bubble, which I don't think we want. I believe these are more examples of the general jankiness you were referring to. What if we remove To add a bubble, onclick of the editor, we can find where in relation to other bubbles the click happened and insert an To delete a bubble without using keyboard navigation, we could do a couple of different things. One option would be to add a cross icon on hover appearing after the text but still within the bubble. Clicking that would remove the bubble. Alternatively, clicking a bubble for the first time could only focus the bubble (from where backspace from above would apply), while the citation dialog would appear on second click. The subtle benefit of this is it would allow the usage of screen readers combined with mouse control (which is impossible now). It's a general idea but let me know if you think it makes sense to explore it further. @dstillman, @stakats - please let me know if you having any input on this suggestion as well. |
|
For some context, that's a very rough demo of what I meant regarding inserting the input field on click and not using the cursor Screen.Recording.2023-10-30.at.5.37.45.PM.mov |
That seems like an excellent way to do it. My plan was always going to be to move them out of the text field completely. It only makes sense for them to be part of the text so that you can insert items in between items, but that is only relevant for some citation styles which do not automatically order cited items anyway, and you can reorder the bubbles by dragging. But this suggestion is actually better.
Given Sean's comments above I think we actually want a dropmarker on the bubbles that is permanent, and adding a delete marker on hover too would clutter things too much.
If I'm understanding correctly you mean opening a bubble would effectively require a double-click? I think that breaks expected behavior from the rest of the UI. How is this behavior impacting usage of screen readers with the mouse? I think backspace on an empty succeeding input field should move the cursor focus onto the bubble and then another backspace should remove it, effectively preserving current behavior. We can also add a remove button in the item popup. |
Great! I'll get started on putting this together.
That is true. Now that I tried it, a dropmarker and a cross do look crowded together, so that's not a great idea.
My thinking was that if you do use a mouse with a screen readers, moving focus on the bubble would cause the screen reader to announce it so that the user gets a confirmation of which bubble they're looking at instead of immediately opening the dialog. I agree, though, it would always require a double-click to open up the dialog which is not optimal. Now that I think about it, if this functionality of just focusing with a mouse click is important, it would be better served with a mouse click when another key (like Shift or Cmd/Ctrl) is being held
That makes sense. I think it would also be appropriate to have Tab/Shift-tab from the input move focus on the next/previous bubble - that also addresses the issue above of getting the focus onto the bubbles while using a mouse.
I think that's a great idea. We have the main dialog for bubbles, might as well make it as functional as possible. As another addition, we could add small buttons to move the bubble left/right (similar to "Move up", "Move down" menuitems for creator rows in itemBox table) to allow reordering of bubbles without relying on drag and drop. |
|
The general approach to remove It does lead to a rewrite of a few other parts tied to the cursor, like drag-and-drop reordering but that's a good opportunity to slightly improve it as well (e.g. I'm adding a placeholder during drag to make it easier to see where the bubble will land). I'm marking this as a draft for now just until all those things are reworked. If anybody has any other examples of jankiness from the dialog not mentioned earlier - feel free to point them out and I'll try to incorporate those fixes as well |
abaevbog
force-pushed
the
quick-format-accessibility
branch
from
7d8b56c
to
4e3b208
Compare
|
@adomasven I think I have a generally working setup without the cursor as proposed earlier. This should work with voiceover, NVDA and JAWS (some notes on that one below). I also tried to address whatever jankiness I ran into with citation dialog and etc. Below is a general outline of my changes under this cursor-less setup and a few questions too. Let me know what you think! I assume there'll be more changes, so I will wait to squash all commits until later Outline of proposed changes
Comments/questions
|
|
General comments with testing:
I imagine it's better for screen readers if focus is shifted to the reference panel, so we should do that, but typing should just move the focus back to input so that the user can continue searching.
I think this is better.
Can we do that just for Windows? VoiceOver is without a doubt the best screen reader and as such I imagine most users with vision impairment use MacOS as other platforms make their lives that much more difficult, so we shouldn't compromise it for JAWS support.
This sounds excellent. I see that dragging not working from input is deliberate, but I don't know why. Possibly because you clear inputs if focus is lost? The logic of clearing inputs should probably change to not do that until a new bubble is added somewhere from a reference panel and we should keep the values of all inputs until then. I imagine this is harder because you have to allow multiple inputs to be displayed and figure out which one the reference panel is being displayed for, but you should reasonably be able to type something, then click on a bubble to check it, then continue typing. I have not looked at the code yet, but I know it was not great before that, so thanks for coming up with a great way to redo the dialog and actually doing it. These changes are awesome overall and going to be a huge improvement to the usability of the dialog and will also probably allow me to reuse this for the eventual citation dialog rewrite. I also wonder if we can get rid of the iframe completely, since as far as I can remember it was only needed because contenteditable didn't work well within the XUL root element, but since we are no longer doing that, this could be simplified. |
|
@adomasven, great feedback! Thank you.
Yeah, in regards to the overall input logic, my thinking was to only allow one thing to be happening at a time: display only one input when one looks for a reference, remove it if the focus moves away, etc. Some jankiness and confusion with the interface earlier came from the fact that some of the actions would conflict with others (e.g. moving the cursor away after typing and checking out the reference panel doesn't quite make sense), so I tried to completely eliminate such possibilities. Maybe a little too much
This certainly should be possible, so I will change to keep the input around until bubble is made or everything is erased.
We certainly can keep all non-empty inputs, and since each input has it's own listener now, re-running search and updating the references list on input focus should not be a problem. I do wonder why would we want to have multiple inputs? I assumed one would not be adding multiple references in parallel jumping between inputs but would do so one bubble at a time. So my inclination would be to keep an input around after blur but still remove it if a user clicks away to an editor to make another input. I admit I may be missing some use case though
There were in fact two reasons for this. First one is me trying to only allow one thing at a time like mentioned earlier. Second one was some odd drag-drop api behavior I encountered. With me removing an input on blur, I blame this open Mozilla bug for not firing dragend event during bubble reordering which made it quite hard to work with, so I intentionally made it not happen. This is mainly a comment on the context, since I will not be hiding the input on blur now, so whatever is the cause of this behaviour, it should not apply.
Fantastic! Having an iframe makes us do some extra work and it also confuses the screen readers a bit since, well, it is a web area, so voiceover announces it as such. I will happily look into gutting it as well. I also wanted to comment on your earlier mention of voiceover getting stuck when opening/closing menupopups: this seems to be an overall issue with voiceover and it happens across all menupopups within the desktop app that I tried. I don't know exactly how we can rectify it yet (since focus does remain where it should) but if there's a solution it should probably be a separate PR to fix all menus if possible. |
Just seems like we should not be discarding user input, although I guess if we're clearing all inputs after bubble addition, clearing an input upon jumping to a different one may not be too much of an issue. An in general this is probably not going to be something common anyway. Also we should make sure that shift-tab from reference panel jumps back to the last active input. |
|
With the input sticking around even if it looses focus, we can have a reference panel opened with the focus being placed on a bubble, say if we open it to check something after typing into the input. In that case, we want ArrowDown to navigate the reference panel, as opposed to my earlier change that triggers the citation dialog to open. @adomasven, I have a few ideas on how to get it working given other changes that we have, and I wanted to ask for your thoughts on which way is best. It would be easy to revert to how it was before, where Ctrl-ArrowDown opens the citation dialog, and ArrowDown navigates the reference panel. However, as I mentioned earlier, JAWS uses Ctrl for its own shortcuts, so that’s not a great option. We could come up with another shortcut of some kind (like Alt-ArrowDown, or something) though I am not a huge fan of shortcuts in general, since it becomes one of 100000 things you need to remember. My next idea is to use Space to trigger the citation dialog when a bubble is focused and leave ArrowDown dedicated to navigation of the reference panel. I like Space because would be consistent with similar uses of Space to trigger a button click throughout the rest of the desktop app, so it doesn't quite count as a shortcut (and a bubble is basically a button, since you click it and it does stuff, plus it now has a dropmarker). However, Space currently is used to create an input after the focused bubble, without which it is impossible to add a reference between bubbles without using a mouse. Maybe some other key could be used to create the input instead of Space? Lastly, I tried something a bit different. What if on ArrowRight/Left from a bubble, we first create and focus the input after/before the current bubble, and on the second arrow click the focus moves to the bubble next to the one where we started? Then, the Space can be allocated to trigger citation dialog on bubble click, and ArrowDown is dedicated to reference panel. These are my thoughts, let me know what you think! In my last commit, I have a tentative implementation of the last idea I tried so you can see what it would look like. (The dialog or bubbles may look slightly different than before - a consequence of iframe removal that I am working on) |
I am having a hard time reproducing this exact behavior. But it really just highlights the jankiness we discussed earlier. In any case, with the changes from this PR, when a bubble is created from a selected item, the |
Why not close the reference panel if a bubble is focused and reopen upon the input becoming active again?
I was surprised that it's not how you decided to implement it initially regarding navigating inputs between bubbles, so this would make sense anyway. We would have to think how we'd close an item popover with the keyboard. It would probably have to be Escape. The only issue is that if the user accidentally double presses Escape the dialog will close along with any changes. |
My reluctance was I didn't think it would be immediately apparent that in order to see the reference panel after you, say, click on a bubble, you have to click back onto the input. Besides it would require a bit more work to see the reference panel again if you rely exclusively on keyboard navigation - one would have to Tab or press a bunch of arrows to reach the input field.
Ok, great. The only reason why I didn't go there right away is because it makes you do more clicks ... Which is really not a concern here given the small number of entries. So I will keep going in this direction and have Space trigger the citation dialog when pressed on a focused bubble. Happy this idea made sense!
By the item popover, you mean the citation properties popup? Escape does work, though Enter achieves the same thing too (and this is what I've been defaulting to). I did notice that sometimes double clicking by accident can cause an unwanted closure of the entire dialog. I thought that maybe we can require a small delay to have passes between two Escape or Enter clicks (e.g. skip all Enter keypresses that come within 2 seconds of the last executed Enter). To tackle the accidental dialog closing we could even go as far as add a confirmation dialog that would pop up if you press Escape when there are some unsaved changes |
|
So in terms of terminology, this is the citation (quick format) dialog, and a citation consists of many items. Given that, I'm calling the item properties popover the "item popover".
Maybe, but even then, if the popover is open, arrow up/down should not navigate the reference panel, and Enter should not select a reference. Generally, after the popover is closed either via mouse or Enter, if there is an input with a value, then the focus should probably jump to that input anyway.
Yeah, both should close it.
Sounds good. |
I am realizing I did get slightly confused with the definitions. Thank you, this actually clears things up for me.
Yes, I tried and when the citation properties popup is show, hiding the reference panel makes it looks cleaner and less busy too
and then on focusout from the the popup for whatever reason (especially important for VoiceOver that moves focus around as it wishes), the focus goes back to the input - I think it's a really good user experience. It also resolves my concerns with it being hard to get back from the citation properties into the input with keyboard as well. |
|
Note for myself to make sure pasting like this works with the new input setup https://forums.zotero.org/discussion/comment/450993/#Comment_450993 |
|
@adomasven, circling back here, this is the summary of my changes after your last feedback. These are all based on your last list and our discussion after it. Let me know what you think!
There are a few minor changes I am wrapping up now but none of them are significant for the overall behavior. These are the final tweaks I will be adding:
|
|
Some general comments:
Overall, after looking at this again, I really dislike the fact that the inputs are cleared. It's just not expected behavior for any UIs to clear inputs without performing some sort of significant context switch, which this isn't. I don't think we can or should make assumptions over what and why people might want to do in this dialog, because in my experience users often have very weird and inefficient workflows that they are "comfortable" and "fast" with and we should allow multiple inputs to have content without clearing them. If it makes thing hard, we could clear the inputs on Enter to add a bubble, but even then we should avoid it if possible. Voiceover:
So my thinking here is if we cannot get Voiceover to announce the selected item in the reference list upon typing, then maybe focus to the reference box should be moved with Tab. If we can, then alt-arrow should navigate the reference list even after it is focused. Also Voiceover should not be able to select the labels/separators in the reference list (Cited, My Library). Overall: fantastic job! The dialog feels much better to use now than it did before, not even mentioning the much improved support for screen readers. |
|
@adomasven, great feedback, thank you!
As I understand, the idea is to provide an efficient workflow, where one doesn't have to spend time to click between multiple inputs, right? Since the actual last input box on Enter will become a bubble, we'll now keep track of the last focused input, as well as the second-to-last input. When an input is turned into a bubble, the focus will go to either the last input (after mouse click) or the second-to-last input (on Enter).
Makes perfect sense! I made that change. The only thing I'm unsure above is the space character. Currently, it is treated as any other character but JAWS by default says "Press whitespace to activate this button", which can be confusing. Should we keep the whitespace as a trigger for bubble popover as well?
Fully agree - I tried it and it's way nicer. Reference panel is shown only when input is active, arrow down opens the popup, arrow up closes it, and the last focused input (if any) is focused when popover closes
I tweaked it a bit to have reasonable margins and to be a bit neater. How is it looking now to you?
Good catch! If the input is too long, it just will move to a separate line and if it overflows that, the textfield will stop expanding. I had quite a few tweaks to drag behavior too, so it should not have any unexpected resizing as well.
I see what you mean with different people having different workflows. Handling multiple inputs was not really an issue: I tried to have nothing cleared automatically and the reference box refreshes when an input is focused, and it feels fairly natural to me.
Yeah, the focus remains on input and getting all screen readers to randomly announce things that are outside of the focus is a bit tricky. It can be a bit confusing too (you do want to hear the screen reader telling you what the input is after all). But Tabbing into the reference panel from the input seems nice to me! I redid the keyboard navigation within the reference panel too to have disabled rows are skipped when the CMD-ArrowUp/ArrowDown navigates the items.
Thank you, half of the merit goes to you and your feedback! I do think it feels more robust than before and is a better experience. We certainly have a few tweaks to take care of but I think we're on the right track! |
|
There are 2 things I wanted to ask about.
Screen.Recording.2024-01-19.at.3.52.09.PM.movThe main issue is that calling
|
abaevbog
force-pushed
the
quick-format-accessibility
branch
from
00b6cf1
to
9dfec43
Compare
|
So I revised the smaller semantics (mainly My intention was to simplify the logic of I'm now going over the "Insert Note" dialog - I didn't realize that it shared |
|
Fixing |
I'm still seeing this. Add multiple bubbles, then type some text in between them, then go to the final position of the input, type and [Enter] to insert a bubble. All the inputs that were in-between bubbles jump to the start of the dialog. Otherwise this is absolutely terrific. Do you think there's anything else to be done here? |
Aha! I I finally ran into it myself. I've been using a citation style without any order requirements and bubble sorting is exactly what caused this. I just pushed a tweak to keep track after which bubble which input went during reordering. Does that fix it for you?
I don't think so ... I'm pretty sure there will be some minor tweaks required down the road since there is a ton of changes but in terms of major stuff, I think it's all in here. I will rebase and squash everything into one commit to keep the description up-to-date. |
- removed all cursor-related logic. Instead, insert a new input component each time the user wants to type. The input remain between bubbles and focusing on one will open the reference panel. Leaving the input hides the reference panel. - during drag-drop reordering, lock the editor height so that it doesn't get out of sync with the window. - removed the iframe, since it was no longer needed. - keyboard navigation: Home/End will place an input at the start/end of the editor and focus it. Tab focuses the last active input if any, or the input in the end otherwise. Shift-Tab from the editor focuses the dropdown button if it is active. Tab from the input will focus the first entry of the reference list. Tab from the reference list will focus the active input. Shift-ArrowLeft/Right from focused bubble will swap the bubble with its neighbor. ArrowDown/Up from bubble will open/close the citation dialog. - when a reference item is selected, previously active input is re-focused. - aria-properties to have voiceover, JAWS and NVDA read bubbles, inputs and reference items, as well as announce hints about available keypresses. - typing from bubble or the reference panel will refocus previously active input - different minor updates to make the functionality less janky - refactoring of refreshing and resizing of the reference panel to be more straightforward and to only do it when necessary. E.g. clicking on a bubble and closing the popover after will not rerun search and just display the old results. - some throttling logic so that two escape keypresses one after another when the itemPopover is open do not close the entire dialog - renamed variables: qfb=dialog, qfe=editor, panel=itemPopover
abaevbog
force-pushed
the
quick-format-accessibility
branch
from
8115bd7
to
dda91b6
Compare
|
@dstillman Do you want to look over the functionality here before merging? |
|
Yep, will do. |
|
Cool! I'll test more later, but a few quick things I noticed (sorry if these are discussed above):
Before:
After:
|
- remove border, etc. from the Z-icon, move dropmarker to the right - remove dropmarker from bubbles to avoid making them wider - clear the input after p. * shortcut
Gotcha - I moved the dropmarker after Z-icon to the right and removed the padding around bubbles. That padding was added for one reason: we had to make some space for the dropmarker to appear in. Other approach that I thought of was to allow the bubble to expand on focus/hover to make space for dropmarker then but it 1. either pushes neighboring bubbles away which looks glitchy 2. or the bubble expands to occupy all available space between itself and the next bubble which makes it hard to click in between bubbles to start typing. I see there are plans to use that space to the right of the bubble for an "X" icon. So I removed the bubbles's dropmarkers altogether for now - correct me if there is a better way to add it without making bubbles wider.
Oh yeah, I did not notice that, thank you. The right edge must have ve been caused by the old iframe setup - i don't see it anymore.
Thank you, it gets cleared now.
Hm, I am actually a bit confused. What exactly is the intended behavior here? When I try this on |
|
This is really great. One more regression I noticed: now, if you clear everything in the box, the search results don't reappear with Open/Selected like they did before. Sorry if this is addressed above, but where did we end up with VoiceOver, NVDA, and JAWS?
Yeah, that shouldn't happen — it should always be "p. 5". We're getting the long form of locator strings from the CSL locales: (and additional lines below that) But we can fix that separately to use the abbreviations for all locators. |
- when only the last, non-removable, input remains, display the reference panel as if the dialog just appeared - add a sanity check to not delete the last non-removable input on blur
itemPopoverLocatorLabel.selectedIndex !== 0 would not allow to select the first locator, Act. Instead, set or remove locator based on the locator's value.
after the redesign-related rebase, the font was too big compared to the old references panel
- when getLocatorString runs the first time, it saved to cache all
forms, including form="short" with a suffix added to the key in the
cache map. E.g. form="short" for page is saved as "page_short".
- when a locatorString is fetched, an additional parameter can be used
to request a specific form, e.g. getLocatorString("page", "short").
If no such form exists, string without any form is returned.
- when the locator string is fetched for a bubble, it asks for the
short locator string, e.g. "p." for page, "para." for paragraph, etc.
Thank you - I added an extra condition to display the item list when everything but the last, non-deletable, input has been removed.
This is the brief summary of how everything is setup now:
In general, at least with my voiceover, NVDA and JAWS, everything works as expected. There are minor peculiarities, of course.
The good thing is that, besides these last point above, voiceover works pretty much the same as JAWS or NVDA, so if you test it on a mac, that should be fairly representative of what other screen-readers would do a well. |
So I went to have a look at how it happens, and I just pushed a suggestion to cache and fetch locator strings by the form as well, so this will use abbreviated versions for all the bubbles. E.g. "p." for "page", "para." for "paragraph" and so on. Is that somewhat what we're after here? |
|
Now that I was looking at the shortcuts (type "p. 5" to add page = 5 to the citation), I also want to make sure their behavior is correct. In the docs, it says I think I got a bit confused because it felt like the locator shortcuts got "stuck" to bubble A. But I guess the idea is that you use this to quickly add a page right after you create a new bubble from the references list, so what I was doing isn't exactly the intended use case. |
Yes, but it's definitely a bug if you click after another citation and it updates a previous one. |
Shortcuts, such as "p. X", or ":X", will always apply X locator value to the bubble right before the input. The only exception is when a new bubble was just added from the reference panel - in this case, the shortcuts will stick to the new bubble. This effect resets on the blur of the input field.
Ok, gotcha! Just wanted to make sure I wasn't misinterpreting anything. I just pushed a change with significantly simplified logic of handling the Correct me if there's anything that's now missing after this update |
Amazing! |
|
Awesome! Very exciting, hopefully this will address the screen readers issues and be a good starting point for further improvements. @adomasven, thank you for guiding me through this! |
|
Excellent! Thanks for taking this on @abaevbog! |
Keyboard navigation and screen reader accessibility for VoiceOver and NVDA.
Changes:
By default, VoiceOver will announce but not select, and NVDA will allow selection but without focus won't read it.
JAWS:
These changes do not make the dialog accessible for JAWS due to the following issues:
<span>is ignored.Some other issues I ran into worth noting:
Notes for testing:
If testing with VoiceOver on a Mac, to move focus to the reference panel from the input field, press Options-ArrowDown, since VoiceOver takes over the usual arrow commands.
Questions:
inputortextareaelement instead ofbodyas the parent for all thespans. Is there a reason whybodywithcontenteditable=trueis currently used, instead ofinputortextarea?