Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extension viewlet scrolls when navigating with tab #44181

Closed
bpasero opened this issue Feb 22, 2018 · 6 comments
Closed

Extension viewlet scrolls when navigating with tab #44181

bpasero opened this issue Feb 22, 2018 · 6 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug verified Verification succeeded workbench-views Workbench view issues
Milestone

Comments

@bpasero
Copy link
Member

bpasero commented Feb 22, 2018

Steps to Reproduce:

  1. have a list with elements inside that have a tab index
  2. navigate to an item that is only partially visible

=> you can end up in a very bad state where the height of the list is no longer proper

before

@bpasero bpasero added the list-widget List widget issues label Feb 22, 2018
@bpasero
Copy link
Member Author

bpasero commented Feb 22, 2018

@joaomoreno another related issue is that the order of DOM elements in the list is not the same order as visible, so that navigation with tab key yields unexpected results. Would it be possible to add a setting to have DOM elements in order? I do not expect a lot of notifications to show up, so I would tolerate a degrade in performance there.

@joaomoreno
Copy link
Member

joaomoreno commented Feb 22, 2018

@bpasero We already established yesterday that this is not a bug in the list. Some element above the parent chain allows the scrolling to happen. We saw the very same thing yesterday in the notifications box.

screen shot 2018-02-22 at 08 55 26

Maybe file another issue for the order?

@joaomoreno joaomoreno added bug Issue identified by VS Code Team member as probable bug and removed list-widget List widget issues labels Feb 22, 2018
@joaomoreno joaomoreno changed the title List: navigating with tab can break the list badly Extension viewlet scrolls when navigating with tab Feb 22, 2018
@joaomoreno joaomoreno added this to the February 2018 milestone Feb 22, 2018
@joaomoreno joaomoreno added the workbench-views Workbench view issues label Feb 22, 2018
@joaomoreno
Copy link
Member

joaomoreno commented Feb 22, 2018

There were two fixes here:

  • Remove the height: auto, introduced by @ramya-rao-a; make sure removing it doesn't break anything. The height of the list always needs to fill its container: 100%.
  • Prevent the monaco-scrollable-element from scrolling itself (it changes its own scrollTop property when navigating to hidden focusable elements) by resetting scrollTop to 0. @alexandrudima should this always be the case for the ScrollableElement class? Or is the list a special case since it implements its own scrolling using translate3d?

@bpasero
Copy link
Member Author

bpasero commented Feb 22, 2018

@joaomoreno I created #44193 for the keyboard accessibility issue.

What is left for me to adopt to get this fix for the notifications? I guess some option to dock list items to the bottom instead of to the top?

@joaomoreno
Copy link
Member

Yeah, sounds about right!

@bpasero bpasero added the verified Verification succeeded label Feb 27, 2018
@alexdima
Copy link
Member

@joaomoreno Yeah, this would happen when not scrolling using the regular scrollTop...

Browsers try to desperately reveal elements that gain focus... scrollTop could be changed for any element in the parent hierarchy for the element which gains focus, until the focused element is no longer occluded.

Here is my version of your listener: https://github.com/Microsoft/vscode/blob/93045738c3112e9b914581b038192b2cb3d4a453/src/vs/editor/browser/viewParts/editorScrollbar/editorScrollbar.ts#L87

@vscodebot vscodebot bot locked and limited conversation to collaborators Apr 8, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug verified Verification succeeded workbench-views Workbench view issues
Projects
None yet
Development

No branches or pull requests

3 participants