[wp-trac] [WordPress Trac] #24048: Code Editors: Increase the usability of Code Editor's files list

[WordPress Trac]

#24048: Code Editors: Increase the usability of Code Editor's files list
--------------------------+--------------------------------
 Reporter:  Daedalon      |       Owner:  westonruter
     Type:  defect (bug)  |      Status:  reopened
 Priority:  high          |   Milestone:  4.9
Component:  Plugins       |     Version:  3.5.1
 Severity:  normal        |  Resolution:
 Keywords:  needs-patch   |     Focuses:  ui, accessibility
--------------------------+--------------------------------
Changes (by afercia):

 * keywords:  has-patch commit => needs-patch
 * status:  closed => reopened
 * resolution:  fixed =>


Comment:

 Found a weird behaviour with Chrome that's pretty serious for keyboard
 navigation.

 Seems `scrollIntoView()` in Chrome is also altering the native tab order.
 To reproduce:
 - just go to the Theme (or plugin) editor page
 - press Tab
 - keyboard navigation should start from the document root, revealing the
 "skip link"
 - instead, keyboard navigation starts from the first file in the tree view
 (highlighted in red in the screenshot below)

 [[Image(https://cldup.com/7YQ0RAcdGs.jpg)]]

 - click on another file in the tree view
 - page reloads
 - press Tab
 - keyboard navigation starts from the editor (focus is inside the editor)

 [[Image(https://cldup.com/Jpzt3Vyq7p.jpg)]]

 Comment the line `this.scrollIntoView( false );` in `wp-admin/js/theme-
 plugin-editor.js` and everything works as expected.

 Couldn't reproduce in other browsers, seems this happens just with Chrome
 (v. 61 both macOS and Windows).

 Seems to me this is really a Chrome bug, I don't see why
 `scrollIntoView()` should alter the native tab sequence. However, it
 should be worked around: starting keyboard navigation form the middle of a
 page shouldn't happen in any case.

 Possible options I can think of:
 - don't use `scrollIntoView()` and use some jQuery scroll animation
 instead
 - use the proprietary `scrollIntoViewIfNeeded()` that seems doesn't
 trigger the bug

 `scrollIntoViewIfNeeded()` is already used for some reason in
 `widgets.js`, I've quickly tried it and seems something like this would
 work:

 {{{
 if ( this.scrollIntoViewIfNeeded ) {
         this.scrollIntoViewIfNeeded();
 } else {
         this.scrollIntoView( false );
 }
 }}}

 Aside: not sure why `.each()` is used
 [https://core.trac.wordpress.org/browser/trunk/src/wp-admin/js/theme-
 plugin-editor.js?rev=41859#L378 here], there's only one `.current-file` at
 a time.

--
Ticket URL: <https://core.trac.wordpress.org/ticket/24048#comment:56>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform