Skip to content

Commit a9fcc43

Browse files
meganroggemeganrogge
authored
Merge pull request microsoft#187077 from hsfzxjy/ctrl-arrow
Generalize Ctrl+DownArrow and Ctrl+UpArrow to most input-result widgets (Fix microsoft#179967)
2 parents 1d96980 + 927510d commit a9fcc43

File tree

11 files changed

+327
-7
lines changed

11 files changed

+327
-7
lines changed

‎src/vs/platform/list/browser/listService.ts‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,14 @@ export class ListService implements IListService {
113113
}
114114
}
115115

116+
export const RawWorkbenchListScrollAtBoundaryContextKey = new RawContextKey<'none' | 'top' | 'bottom' | 'both'>('listScrollAtBoundary', 'none');
117+
export const WorkbenchListScrollAtTopContextKey = ContextKeyExpr.or(
118+
RawWorkbenchListScrollAtBoundaryContextKey.isEqualTo('top'),
119+
RawWorkbenchListScrollAtBoundaryContextKey.isEqualTo('both'));
120+
export const WorkbenchListScrollAtBottomContextKey = ContextKeyExpr.or(
121+
RawWorkbenchListScrollAtBoundaryContextKey.isEqualTo('bottom'),
122+
RawWorkbenchListScrollAtBoundaryContextKey.isEqualTo('both'));
123+
116124
export const RawWorkbenchListFocusContextKey = new RawContextKey<boolean>('listFocus', true);
117125
export const WorkbenchListSupportsMultiSelectContextKey = new RawContextKey<boolean>('listSupportsMultiselect', true);
118126
export const WorkbenchListFocusContextKey = ContextKeyExpr.and(RawWorkbenchListFocusContextKey, ContextKeyExpr.not(InputFocusedContextKey));
@@ -139,6 +147,33 @@ function createScopedContextKeyService(contextKeyService: IContextKeyService, wi
139147
return result;
140148
}
141149

150+
// Note: We must declare IScrollObservarable as the arithmetic of concrete classes,
151+
// instead of object type like { onDidScroll: Event<any>; ... }. The latter will not mark
152+
// those properties as referenced during tree-shaking, causing them to be shaked away.
153+
type IScrollObservarable = Exclude<WorkbenchListWidget, WorkbenchPagedList<any>> | List<any>;
154+
155+
function createScrollObserver(contextKeyService: IContextKeyService, widget: IScrollObservarable): IDisposable {
156+
const listScrollAt = RawWorkbenchListScrollAtBoundaryContextKey.bindTo(contextKeyService);
157+
const update = () => {
158+
const atTop = widget.scrollTop === 0;
159+
160+
// We need a threshold `1` since scrollHeight is rounded.
161+
// https://developer.mozilla.org/en-US/docs/Web/API/Element/scrollHeight#determine_if_an_element_has_been_totally_scrolled
162+
const atBottom = widget.scrollHeight - widget.renderHeight - widget.scrollTop < 1;
163+
if (atTop && atBottom) {
164+
listScrollAt.set('both');
165+
} else if (atTop) {
166+
listScrollAt.set('top');
167+
} else if (atBottom) {
168+
listScrollAt.set('bottom');
169+
} else {
170+
listScrollAt.set('none');
171+
}
172+
};
173+
update();
174+
return widget.onDidScroll(update);
175+
}
176+
142177
const multiSelectModifierSettingKey = 'workbench.list.multiSelectModifier';
143178
const openModeSettingKey = 'workbench.list.openMode';
144179
const horizontalScrollingKey = 'workbench.list.horizontalScrolling';
@@ -259,6 +294,8 @@ export class WorkbenchList<T> extends List<T> {
259294

260295
this.contextKeyService = createScopedContextKeyService(contextKeyService, this);
261296

297+
this.disposables.add(createScrollObserver(this.contextKeyService, this));
298+
262299
this.listSupportsMultiSelect = WorkbenchListSupportsMultiSelectContextKey.bindTo(this.contextKeyService);
263300
this.listSupportsMultiSelect.set(options.multipleSelectionSupport !== false);
264301

@@ -390,6 +427,8 @@ export class WorkbenchPagedList<T> extends PagedList<T> {
390427

391428
this.contextKeyService = createScopedContextKeyService(contextKeyService, this);
392429

430+
this.disposables.add(createScrollObserver(this.contextKeyService, this.widget));
431+
393432
this.horizontalScrolling = options.horizontalScrolling;
394433

395434
this.listSupportsMultiSelect = WorkbenchListSupportsMultiSelectContextKey.bindTo(this.contextKeyService);
@@ -514,6 +553,8 @@ export class WorkbenchTable<TRow> extends Table<TRow> {
514553

515554
this.contextKeyService = createScopedContextKeyService(contextKeyService, this);
516555

556+
this.disposables.add(createScrollObserver(this.contextKeyService, this));
557+
517558
this.listSupportsMultiSelect = WorkbenchListSupportsMultiSelectContextKey.bindTo(this.contextKeyService);
518559
this.listSupportsMultiSelect.set(options.multipleSelectionSupport !== false);
519560

@@ -1165,6 +1206,8 @@ class WorkbenchTreeInternals<TInput, T, TFilterData> {
11651206
) {
11661207
this.contextKeyService = createScopedContextKeyService(contextKeyService, tree);
11671208

1209+
this.disposables.push(createScrollObserver(this.contextKeyService, tree));
1210+
11681211
this.listSupportsMultiSelect = WorkbenchListSupportsMultiSelectContextKey.bindTo(this.contextKeyService);
11691212
this.listSupportsMultiSelect.set(options.multipleSelectionSupport !== false);
11701213

‎src/vs/workbench/browser/actions/listCommands.ts‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import { KeyMod, KeyCode } from 'vs/base/common/keyCodes';
77
import { ServicesAccessor } from 'vs/platform/instantiation/common/instantiation';
88
import { KeybindingsRegistry, KeybindingWeight } from 'vs/platform/keybinding/common/keybindingsRegistry';
99
import { List } from 'vs/base/browser/ui/list/listWidget';
10-
import { WorkbenchListFocusContextKey, IListService, WorkbenchListSupportsMultiSelectContextKey, ListWidget, WorkbenchListHasSelectionOrFocus, getSelectionKeyboardEvent, WorkbenchListWidget, WorkbenchListSelectionNavigation, WorkbenchTreeElementCanCollapse, WorkbenchTreeElementHasParent, WorkbenchTreeElementHasChild, WorkbenchTreeElementCanExpand, RawWorkbenchListFocusContextKey, WorkbenchTreeFindOpen, WorkbenchListSupportsFind } from 'vs/platform/list/browser/listService';
10+
import { WorkbenchListFocusContextKey, IListService, WorkbenchListSupportsMultiSelectContextKey, ListWidget, WorkbenchListHasSelectionOrFocus, getSelectionKeyboardEvent, WorkbenchListWidget, WorkbenchListSelectionNavigation, WorkbenchTreeElementCanCollapse, WorkbenchTreeElementHasParent, WorkbenchTreeElementHasChild, WorkbenchTreeElementCanExpand, RawWorkbenchListFocusContextKey, WorkbenchTreeFindOpen, WorkbenchListSupportsFind, WorkbenchListScrollAtBottomContextKey, WorkbenchListScrollAtTopContextKey } from 'vs/platform/list/browser/listService';
1111
import { PagedList } from 'vs/base/browser/ui/list/listPaging';
1212
import { equals, range } from 'vs/base/common/arrays';
1313
import { ContextKeyExpr } from 'vs/platform/contextkey/common/contextkey';
@@ -692,7 +692,12 @@ KeybindingsRegistry.registerCommandAndKeybindingRule({
692692
KeybindingsRegistry.registerCommandAndKeybindingRule({
693693
id: 'list.scrollUp',
694694
weight: KeybindingWeight.WorkbenchContrib,
695-
when: WorkbenchListFocusContextKey,
695+
// Since the default keybindings for list.scrollUp and widgetNavigation.focusPrevious
696+
// are both Ctrl+UpArrow, we disable this command when the scrollbar is at
697+
// top-most position. This will give chance for widgetNavigation.focusPrevious to execute
698+
when: ContextKeyExpr.and(
699+
WorkbenchListFocusContextKey,
700+
WorkbenchListScrollAtTopContextKey?.negate()),
696701
primary: KeyMod.CtrlCmd | KeyCode.UpArrow,
697702
handler: accessor => {
698703
const focused = accessor.get(IListService).lastFocusedList;
@@ -708,7 +713,10 @@ KeybindingsRegistry.registerCommandAndKeybindingRule({
708713
KeybindingsRegistry.registerCommandAndKeybindingRule({
709714
id: 'list.scrollDown',
710715
weight: KeybindingWeight.WorkbenchContrib,
711-
when: WorkbenchListFocusContextKey,
716+
// same as above
717+
when: ContextKeyExpr.and(
718+
WorkbenchListFocusContextKey,
719+
WorkbenchListScrollAtBottomContextKey?.negate()),
712720
primary: KeyMod.CtrlCmd | KeyCode.DownArrow,
713721
handler: accessor => {
714722
const focused = accessor.get(IListService).lastFocusedList;

‎src/vs/workbench/browser/actions/widgetNavigationCommands.ts‎

Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,149 @@
1+
/*---------------------------------------------------------------------------------------------
2+
* Copyright (c) Microsoft Corporation. All rights reserved.
3+
* Licensed under the MIT License. See License.txt in the project root for license information.
4+
*--------------------------------------------------------------------------------------------*/
5+
6+
import { KeyMod, KeyCode } from 'vs/base/common/keyCodes';
7+
import { ContextKeyExpr, IContextKey, IContextKeyService, RawContextKey } from 'vs/platform/contextkey/common/contextkey';
8+
import { KeybindingWeight, KeybindingsRegistry } from 'vs/platform/keybinding/common/keybindingsRegistry';
9+
import { WorkbenchListFocusContextKey, WorkbenchListScrollAtBottomContextKey, WorkbenchListScrollAtTopContextKey } from 'vs/platform/list/browser/listService';
10+
import { Event } from 'vs/base/common/event';
11+
import { combinedDisposable, toDisposable, IDisposable, Disposable } from 'vs/base/common/lifecycle';
12+
import { Registry } from 'vs/platform/registry/common/platform';
13+
import { IWorkbenchContributionsRegistry, Extensions as WorkbenchExtensions } from 'vs/workbench/common/contributions';
14+
import { LifecyclePhase } from 'vs/workbench/services/lifecycle/common/lifecycle';
15+
16+
/** INavigatableContainer represents a logical container composed of widgets that can
17+
be navigated back and forth with key shortcuts */
18+
interface INavigatableContainer {
19+
/**
20+
* The container may coomposed of multiple parts that share no DOM ancestor
21+
* (e.g., the main body and filter box of MarkersView may be separated).
22+
* To track the focus of container we must pass in focus/blur events of all parts
23+
* as `focusNotifiers`.
24+
*
25+
* Each element of `focusNotifiers` notifies the focus/blur event for a part of
26+
* the container. The container is considered focused if at least one part being
27+
* focused, and blurred if all parts being blurred.
28+
*/
29+
readonly focusNotifiers: readonly IFocusNotifier[];
30+
focusPreviousWidget(): void;
31+
focusNextWidget(): void;
32+
}
33+
34+
interface IFocusNotifier {
35+
readonly onDidFocus: Event<any>;
36+
readonly onDidBlur: Event<any>;
37+
}
38+
39+
function handleFocusEventsGroup(group: readonly IFocusNotifier[], handler: (isFocus: boolean) => void): IDisposable {
40+
const focusedIndices = new Set<number>();
41+
return combinedDisposable(...group.map((events, index) => combinedDisposable(
42+
events.onDidFocus(() => {
43+
if (!focusedIndices.size) {
44+
handler(true);
45+
}
46+
focusedIndices.add(index);
47+
}),
48+
events.onDidBlur(() => {
49+
focusedIndices.delete(index);
50+
if (!focusedIndices.size) {
51+
handler(false);
52+
}
53+
}),
54+
)));
55+
}
56+
57+
const NavigatableContainerFocusedContextKey = new RawContextKey<boolean>('navigatableContainerFocused', false);
58+
59+
class NavigatableContainerManager implements IDisposable {
60+
private static INSTANCE: NavigatableContainerManager | undefined;
61+
62+
private readonly containers = new Set<INavigatableContainer>();
63+
private lastContainer: INavigatableContainer | undefined;
64+
private focused: IContextKey<boolean>;
65+
66+
67+
constructor(@IContextKeyService contextKeyService: IContextKeyService) {
68+
this.focused = NavigatableContainerFocusedContextKey.bindTo(contextKeyService);
69+
NavigatableContainerManager.INSTANCE = this;
70+
}
71+
72+
dispose(): void {
73+
this.containers.clear();
74+
this.focused.reset();
75+
NavigatableContainerManager.INSTANCE = undefined;
76+
}
77+
78+
static register(container: INavigatableContainer): IDisposable {
79+
const instance = this.INSTANCE;
80+
if (!instance) {
81+
return Disposable.None;
82+
}
83+
instance.containers.add(container);
84+
85+
return combinedDisposable(
86+
handleFocusEventsGroup(container.focusNotifiers, (isFocus) => {
87+
if (isFocus) {
88+
instance.focused.set(true);
89+
instance.lastContainer = container;
90+
} else if (instance.lastContainer === container) {
91+
instance.focused.set(false);
92+
instance.lastContainer = undefined;
93+
}
94+
}),
95+
toDisposable(() => {
96+
instance.containers.delete(container);
97+
if (instance.lastContainer === container) {
98+
instance.focused.set(false);
99+
instance.lastContainer = undefined;
100+
}
101+
})
102+
);
103+
}
104+
105+
static getActive(): INavigatableContainer | undefined {
106+
return this.INSTANCE?.lastContainer;
107+
}
108+
}
109+
110+
export function registerNavigatableContainer(container: INavigatableContainer): IDisposable {
111+
return NavigatableContainerManager.register(container);
112+
}
113+
114+
Registry.as<IWorkbenchContributionsRegistry>(WorkbenchExtensions.Workbench)
115+
.registerWorkbenchContribution(NavigatableContainerManager, LifecyclePhase.Starting);
116+
117+
KeybindingsRegistry.registerCommandAndKeybindingRule({
118+
id: 'widgetNavigation.focusPrevious',
119+
weight: KeybindingWeight.WorkbenchContrib,
120+
when: ContextKeyExpr.and(
121+
NavigatableContainerFocusedContextKey,
122+
ContextKeyExpr.or(
123+
WorkbenchListFocusContextKey?.negate(),
124+
WorkbenchListScrollAtTopContextKey,
125+
)
126+
),
127+
primary: KeyMod.CtrlCmd | KeyCode.UpArrow,
128+
handler: () => {
129+
const activeContainer = NavigatableContainerManager.getActive();
130+
activeContainer?.focusPreviousWidget();
131+
}
132+
});
133+
134+
KeybindingsRegistry.registerCommandAndKeybindingRule({
135+
id: 'widgetNavigation.focusNext',
136+
weight: KeybindingWeight.WorkbenchContrib,
137+
when: ContextKeyExpr.and(
138+
NavigatableContainerFocusedContextKey,
139+
ContextKeyExpr.or(
140+
WorkbenchListFocusContextKey?.negate(),
141+
WorkbenchListScrollAtBottomContextKey,
142+
)
143+
),
144+
primary: KeyMod.CtrlCmd | KeyCode.DownArrow,
145+
handler: () => {
146+
const activeContainer = NavigatableContainerManager.getActive();
147+
activeContainer?.focusNextWidget();
148+
}
149+
});

‎src/vs/workbench/browser/parts/views/viewFilter.ts‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,10 @@ export class FilterWidget extends Widget {
8181
private moreFiltersActionViewItem: MoreFiltersActionViewItem | undefined;
8282
private isMoreFiltersChecked: boolean = false;
8383

84+
private focusTracker: DOM.IFocusTracker;
85+
public get onDidFocus() { return this.focusTracker.onDidFocus; }
86+
public get onDidBlur() { return this.focusTracker.onDidBlur; }
87+
8488
constructor(
8589
private readonly options: IFilterWidgetOptions,
8690
@IInstantiationService private readonly instantiationService: IInstantiationService,
@@ -97,7 +101,7 @@ export class FilterWidget extends Widget {
97101
}
98102

99103
this.element = DOM.$('.viewpane-filter');
100-
this.filterInputBox = this.createInput(this.element);
104+
[this.filterInputBox, this.focusTracker] = this.createInput(this.element);
101105

102106
const controlsContainer = DOM.append(this.element, DOM.$('.viewpane-filter-controls'));
103107
this.filterBadge = this.createBadge(controlsContainer);
@@ -106,6 +110,10 @@ export class FilterWidget extends Widget {
106110
this.adjustInputBox();
107111
}
108112

113+
hasFocus(): boolean {
114+
return this.filterInputBox.hasFocus();
115+
}
116+
109117
focus(): void {
110118
this.filterInputBox.focus();
111119
}
@@ -145,7 +153,7 @@ export class FilterWidget extends Widget {
145153
}
146154
}
147155

148-
private createInput(container: HTMLElement): ContextScopedHistoryInputBox {
156+
private createInput(container: HTMLElement): [ContextScopedHistoryInputBox, DOM.IFocusTracker] {
149157
const inputBox = this._register(this.instantiationService.createInstance(ContextScopedHistoryInputBox, container, this.contextViewService, {
150158
placeholder: this.options.placeholder,
151159
ariaLabel: this.options.ariaLabel,
@@ -171,7 +179,7 @@ export class FilterWidget extends Widget {
171179
this._register(focusTracker.onDidBlur(() => this.focusContextKey!.set(false)));
172180
this._register(toDisposable(() => this.focusContextKey!.reset()));
173181
}
174-
return inputBox;
182+
return [inputBox, focusTracker];
175183
}
176184

177185
private createBadge(container: HTMLElement): HTMLElement {

‎src/vs/workbench/contrib/comments/browser/commentsView.ts‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ import { ITreeElement } from 'vs/base/browser/ui/tree/tree';
4242
import { Iterable } from 'vs/base/common/iterator';
4343
import { CommentController } from 'vs/workbench/contrib/comments/browser/commentsController';
4444
import { Range } from 'vs/editor/common/core/range';
45+
import { registerNavigatableContainer } from 'vs/workbench/browser/actions/widgetNavigationCommands';
4546

4647
const CONTEXT_KEY_HAS_COMMENTS = new RawContextKey<boolean>('commentsView.hasComments', false);
4748
const CONTEXT_KEY_SOME_COMMENTS_EXPANDED = new RawContextKey<boolean>('commentsView.someCommentsExpanded', false);
@@ -145,6 +146,23 @@ export class CommentsPanel extends FilterViewPane implements ICommentsView {
145146
super.saveState();
146147
}
147148

149+
override render(): void {
150+
super.render();
151+
this._register(registerNavigatableContainer({
152+
focusNotifiers: [this, this.filterWidget],
153+
focusNextWidget: () => {
154+
if (this.filterWidget.hasFocus()) {
155+
this.focus();
156+
}
157+
},
158+
focusPreviousWidget: () => {
159+
if (!this.filterWidget.hasFocus()) {
160+
this.focusFilter();
161+
}
162+
}
163+
}));
164+
}
165+
148166
public focusFilter(): void {
149167
this.filterWidget.focus();
150168
}

‎src/vs/workbench/contrib/debug/browser/repl.ts‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ import { CONTEXT_DEBUG_STATE, CONTEXT_IN_DEBUG_REPL, CONTEXT_MULTI_SESSION_REPL,
6868
import { Variable } from 'vs/workbench/contrib/debug/common/debugModel';
6969
import { ReplEvaluationResult, ReplGroup } from 'vs/workbench/contrib/debug/common/replModel';
7070
import { IEditorService } from 'vs/workbench/services/editor/common/editorService';
71+
import { registerNavigatableContainer } from 'vs/workbench/browser/actions/widgetNavigationCommands';
7172

7273
const $ = dom.$;
7374

@@ -563,6 +564,27 @@ export class Repl extends FilterViewPane implements IHistoryNavigationWidget {
563564

564565
// --- Creation
565566

567+
override render(): void {
568+
super.render();
569+
this._register(registerNavigatableContainer({
570+
focusNotifiers: [this, this.filterWidget],
571+
focusNextWidget: () => {
572+
if (this.filterWidget.hasFocus()) {
573+
this.tree?.domFocus();
574+
} else if (this.tree?.getHTMLElement() === document.activeElement) {
575+
this.focus();
576+
}
577+
},
578+
focusPreviousWidget: () => {
579+
if (this.replInput.hasTextFocus()) {
580+
this.tree?.domFocus();
581+
} else if (this.tree?.getHTMLElement() === document.activeElement) {
582+
this.focusFilter();
583+
}
584+
}
585+
}));
586+
}
587+
566588
protected override renderBody(parent: HTMLElement): void {
567589
super.renderBody(parent);
568590
this.container = dom.append(parent, $('.repl'));

0 commit comments

Comments
 (0)