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

Fix tabbing in trees #32502

Merged
merged 2 commits into from
Aug 15, 2017
Merged

Fix tabbing in trees #32502

merged 2 commits into from
Aug 15, 2017

Conversation

Lixire
Copy link
Contributor

@Lixire Lixire commented Aug 14, 2017

PR 2: Electric Boogaloo

I changed the tab index to -1 instead of not setting it at all so it can still be focusable which fixes the quick navigate feature.

  • Changes the tabbing in quick-open-tree so that the actionbar items can be reached in one tab instead of two (where the first tab was to focus the whole tree).
  • Keeps quickopen open when pressing up or down with an action bar item focused

Fixes #31835 and #31836

@Lixire Lixire added terminal Integrated terminal issues quick-pick Quick-pick widget issues labels Aug 14, 2017
@Lixire Lixire added this to the August 2017 milestone Aug 14, 2017
@Lixire Lixire self-assigned this Aug 14, 2017
@@ -444,7 +444,7 @@ export class TreeView extends HeightMap {

this.domNode = document.createElement('div');
this.domNode.className = `monaco-tree no-focused-item monaco-tree-instance-${this.instance}`;
this.domNode.tabIndex = 0;
this.domNode.tabIndex = context.options.preventRootFocus ? -1 : 0;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is preventRootFocus still a good name for the option? And were you able to figure out why we need the tabIndex for quick navigate?

Copy link
Member

@bpasero bpasero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, seems to work nicely 👍

@@ -335,7 +336,15 @@ export class QuickOpenWidget implements IModelProvider {
}

this.applyStyles();

Copy link
Member

@bpasero bpasero Aug 15, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Lixire I prefer to keep the newline here, looks logically too close otherwise imho:

image

@roblourens roblourens merged commit 3cd2d7c into master Aug 15, 2017
@bpasero bpasero deleted the amqi/tree-tabs branch August 15, 2017 17:56
@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
quick-pick Quick-pick widget issues terminal Integrated terminal issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Terminal Switcher: Rename should be reachable with first tab
4 participants