Reduce the number of tabstops on the Markdown toolbar
The following discussion from !34300 should be addressed: >>> Thanks for this @cngo! The preexisting `tabindex="-1"` make me think there might be some preexisting intention here. Either way, we should probably let UX take a look :smile: @pedroms I've assign this to you. Could you please take a look at this small MR which removes some `tabindex="-1"` from some elements? I also raised a specific question for you https://gitlab.com/gitlab-org/gitlab/-/merge_requests/34300#note_362375072. Thanks! :bow: Robert Speicher Robert Speicher 🏝 @rspeicher · 6 days ago Owner @cngo @pslaughter There was intention here. Tabstops should go directly from the title field to the description field. If we want to make them tab accessible, their tabindex should be higher than the description field. Additionally, I now have to tab through 11 fields to get the description due to the toolbar. --- Paul Slaughter Paul Slaughter 🔴 @pslaughter · 5 days ago Maintainer @pedroms can you please address this concern @rspeicher raised? --- Robert Speicher Robert Speicher 🏝 @rspeicher · 5 days ago Owner For myself, I've solved this with a TamperMonkey script: ```javascript document.querySelectorAll('.md-header button').forEach(e => e.setAttribute('tabIndex', -1)); ``` --- Paul Slaughter Paul Slaughter 🔴 @pslaughter · 5 days ago Maintainer If we're still wanting to make the toolbar buttons tab-able, the best workaround I can see for going from "title" to "description text area" is to reverse the order of the HTML elements, but use flex to present this properly. Something like... ```diff diff --git a/app/views/shared/_md_preview.html.haml b/app/views/shared/_md_preview.html.haml index c3818b9f7ae..a97ad6758d5 100644 --- a/app/views/shared/_md_preview.html.haml +++ b/app/views/shared/_md_preview.html.haml @@ -7,7 +7,10 @@ = _('This merge request is locked.') = _('Only project members can comment.') -.md-area.position-relative +.md-area.position-relative.d-flex.flex-column-reverse + .md-write-holder + = yield + .md.md-preview-holder.js-md-preview.hide{ data: { url: url } } .md-header %ul.nav.nav-tabs.nav-links.clearfix %li.md-header-tab.active @@ -20,9 +23,6 @@ %li.md-header-toolbar.active = render 'shared/blob/markdown_buttons', show_fullscreen_button: true - .md-write-holder - = yield - .md.md-preview-holder.js-md-preview.hide{ data: { url: url } } .referenced-commands.hide - if referenced_users ``` What do you think @cngo? --- Paul Slaughter Paul Slaughter 🔴 @pslaughter · 5 days ago Maintainer @rspeicher do you think this a significant enough user frustration that we should revert? --- Robert Speicher Robert Speicher 🏝 @rspeicher · 5 days ago Owner @pslaughter Unclear. So far I'm the only one to complain AFAIK. 😃 I would think we could also give the title and description tabindex values of 1, and then the rest a value of 2. Though http://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/tabindex recommends not using a value greater than 0. 🤔 https://webaim.org/techniques/keyboard/tabindex seems to have a similar recommendation. Edited by Robert Speicher 5 days ago --- Paul Slaughter Paul Slaughter 🔴 @pslaughter · 5 days ago Maintainer Though http://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/tabindex recommends not using a value greater than 0. Yeah, managing tabindex values can be like managing z-index values and introduces a lot of global coupling. AFAIK, it's better to semantically order elements in order of "importance" and use CSS to change the presentation if necessary. So far I'm the only one to complain AFAIK. 😃 As the "first complainer" 😉 would you mind creating an issue? I'm sure some other users will share your opinions and would be nice to capture this. --- Pedro Moreira da Silva Pedro Moreira da Silva @pedroms · 5 days ago Developer @rspeicher thanks for raising that concern, I understand. See https://www.w3.org/TR/wai-aria-practices/examples/toolbar/toolbar.html for an alternative approach (by the way, this is the approach that GitHub uses). The approach is to set tab-index="0" just for the first toolbar button, and the other toolbar buttons have tab-index"-1". Then, to navigate through the toolbar, you use the keyboard right and left arrows, which toggle the button's tabindex between 0 and -1. With this toggling, it means that you can tab back and forth between the textarea and the toolbar, landing on the last toolbar button that you had selected. With this approach, it only takes one more tab to land on the issue/MR/epic description. @pslaughter I think this would be a great improvement, but I'm assuming it would be a significant effort? --- Coung Ngo Coung Ngo @cngo · 1 day ago Developer Sorry I was OOO last week. There might have been intention here but from my perspective it appears to only benefit able-bodied users while removing features for other users. We should provide equivalent experiences to all types of users who use GitLab. An able-bodied user can click on any of the toolbar buttons or Preview but how can non-mouse users perform the same action if these elements are not tab accessible? Conversely, if we were to keep the toolbar buttons tab-inaccessible then does that mean we don't actually value the buttons as useful and therefore they should be removed from the comment textarea? I agree that tabbing through 11 fields to get from title to description is annoying (I frequently use tab too) but able-bodied users can switch to using the mouse to get to description faster (though of course not as fast as pressing the tab key once). I would argue that this inconvenience is minor compared to how non-mouse users are now able to access features everyone else has been using all along. And developers have the privilege of using tools like Tampermonkey to change their web experience to what suits them, which others unfortunately do not have. @pslaughter Thanks for the suggestion. However, I don't think we should re-order the HTML to change the tab order to visually go from the title to the description then back up to the toolbar as this does not follow the flow of elements on the page. I don't think we need a workaround, but any future work can improve on the tabbing experience with what @pedroms suggested by making the tab stops go from from Title -> Write/Preview -> Toolbar -> Description and using the left/right arrow keys to switch between the Write/Preview and Toolbar buttons. >>>
issue