Skip to content

Commit 9b1a541

Browse files
author
Benjamin Pasero
committed
notifications - better keyboard support
1 parent 9a3b1b4 commit 9b1a541

6 files changed

Lines changed: 33 additions & 19 deletions

File tree

‎src/vs/base/browser/ui/button/button.css‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,4 @@
2020
.monaco-button.disabled {
2121
opacity: 0.4;
2222
cursor: default;
23-
}
24-
25-
/* Theming support */
26-
27-
.vs .monaco-text-button:focus,
28-
.vs-dark .monaco-text-button:focus {
29-
outline-color: rgba(255, 255, 255, .5); /* buttons have a blue color, so focus indication needs to be different */
3023
}

‎src/vs/base/browser/ui/button/button.ts‎

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,8 @@ export class Button {
4343
private _onDidClick = new Emitter<any>();
4444
readonly onDidClick: Event<any> = this._onDidClick.event;
4545

46+
private focusTracker: DOM.IFocusTracker;
47+
4648
constructor(container: Builder, options?: IButtonOptions);
4749
constructor(container: HTMLElement, options?: IButtonOptions);
4850
constructor(container: any, options?: IButtonOptions) {
@@ -86,20 +88,29 @@ export class Button {
8688

8789
this.$el.on(DOM.EventType.MOUSE_OVER, (e) => {
8890
if (!this.$el.hasClass('disabled')) {
89-
const hoverBackground = this.buttonHoverBackground ? this.buttonHoverBackground.toString() : null;
90-
if (hoverBackground) {
91-
this.$el.style('background-color', hoverBackground);
92-
}
91+
this.setHoverBackground();
9392
}
9493
});
9594

9695
this.$el.on(DOM.EventType.MOUSE_OUT, (e) => {
9796
this.applyStyles(); // restore standard styles
9897
});
9998

99+
// Also set hover background when button is focused for feedback
100+
const tracker = DOM.trackFocus(this.$el.getHTMLElement());
101+
tracker.onDidFocus(() => this.setHoverBackground());
102+
tracker.onDidBlur(() => this.applyStyles()); // restore standard styles
103+
100104
this.applyStyles();
101105
}
102106

107+
private setHoverBackground(): void {
108+
const hoverBackground = this.buttonHoverBackground ? this.buttonHoverBackground.toString() : null;
109+
if (hoverBackground) {
110+
this.$el.style('background-color', hoverBackground);
111+
}
112+
}
113+
103114
style(styles: IButtonStyles): void {
104115
this.buttonForeground = styles.buttonForeground;
105116
this.buttonBackground = styles.buttonBackground;
@@ -165,6 +176,9 @@ export class Button {
165176
if (this.$el) {
166177
this.$el.dispose();
167178
this.$el = null;
179+
180+
this.focusTracker.dispose();
181+
this.focusTracker = null;
168182
}
169183

170184
this._onDidClick.dispose();

‎src/vs/workbench/browser/parts/editor/media/editorstatus.css‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@
3535
position: absolute;
3636
top: 0;
3737
right: 0;
38-
margin: .5em 0 0;
3938
padding: .5em;
4039
width: 22px;
4140
height: 22px;
@@ -67,7 +66,6 @@
6766
padding-left: 12px;
6867
padding-right: 12px;
6968
border: 4px solid #007ACC;
70-
border-radius: 4px;
7169
float: left;
7270
margin-right: 5px;
7371
}

‎src/vs/workbench/browser/parts/notifications/media/notificationsCenter.css‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
.monaco-workbench > .notifications-list-container .notification-list-item {
2828
display: flex;
2929
flex-direction: column;
30+
flex-direction: column-reverse; /* the details row appears first in order for better keyboard access to notification buttons */
3031
padding: 10px 5px;
3132
height: 100%;
3233
box-sizing: border-box;
@@ -82,6 +83,7 @@
8283
}
8384

8485
.monaco-workbench > .notifications-list-container .notification-list-item:hover .notification-list-item-toolbar-container,
86+
.monaco-workbench > .notifications-list-container .monaco-list-row.focused .notification-list-item .notification-list-item-toolbar-container,
8587
.monaco-workbench > .notifications-list-container .notification-list-item.expanded .notification-list-item-toolbar-container {
8688
display: block;
8789
}

‎src/vs/workbench/browser/parts/notifications/notificationsCenter.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -198,8 +198,8 @@ export class NotificationsCenter extends Themable {
198198
this.hide();
199199
}
200200

201-
// Otherwise restore focus
202-
else {
201+
// Otherwise restore focus if we had
202+
else if (typeof focusedIndex === 'number') {
203203
let indexToFocus = 0;
204204
if (focusedItem) {
205205
let indexToFocusCandidate = this.viewModel.indexOf(focusedItem);

‎src/vs/workbench/browser/parts/notifications/notificationsViewer.ts‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -225,15 +225,17 @@ export class NotificationRenderer implements IRenderer<INotificationViewItem, IN
225225

226226
container.appendChild(data.container);
227227

228+
// the details row appears first in order for better keyboard access to notification buttons
229+
data.container.appendChild(data.detailsRow);
230+
data.detailsRow.appendChild(data.source);
231+
data.detailsRow.appendChild(data.actionsContainer);
232+
233+
// main row
228234
data.container.appendChild(data.mainRow);
229235
data.mainRow.appendChild(data.icon);
230236
data.mainRow.appendChild(data.message);
231237
data.mainRow.appendChild(toolbarContainer);
232238

233-
data.container.appendChild(data.detailsRow);
234-
data.detailsRow.appendChild(data.source);
235-
data.detailsRow.appendChild(data.actionsContainer);
236-
237239
return data;
238240
}
239241

@@ -263,6 +265,11 @@ export class NotificationRenderer implements IRenderer<INotificationViewItem, IN
263265
clearNode(data.message);
264266
data.message.appendChild(NotificationMessageMarkdownRenderer.render(notification.message, (content: string) => this.openerService.open(URI.parse(content)).then(void 0, onUnexpectedError)));
265267

268+
const links = data.message.querySelectorAll('a');
269+
for (let i = 0; i < links.length; i++) {
270+
links.item(i).tabIndex = -1; // prevent keyboard navigation to links to allow for better keyboard support within a message
271+
}
272+
266273
// Actions
267274
const actions: IAction[] = [];
268275

0 commit comments

Comments
 (0)