Skip to content

Commit 513b849

Browse files
Update/positron list focus and selection (#13707)
# PositronList focus & selection + supporting changes This PR adds focus and selection behavior to `PositronList`, and includes a handful of supporting improvements to `DataGridInstance`, the custom context menu, and the Data Connections panel that landed along the way. ## Selection mode on DataGridInstance Introduces `selectionMode: 'spreadsheet' | 'list'` as a discriminated option (rejected at compile time when `selection: false`): - **`'spreadsheet'`** (default): plain keyboard navigation clears the selection — cursor and selection are independent. Existing main data grid and table summary behavior is preserved. - **`'list'`**: plain keyboard navigation re-selects the cursor row after the move so single-select tracks the cursor. Shift-navigation still extends selection regardless of mode. The waffle's navigation handlers (ArrowUp/Down/Left/Right, Home, End, Cmd+Home, Cmd+End, PageUp, PageDown) all route through a small `navigationSelection.{beforeMoveCursor, afterMoveCursor}` helper so the policy lives in one place. ## PositronList focus & selection - `PositronListInstance` opts into `selectionMode: 'list'` so plain arrow navigation moves the row selection together with the cursor. - Cell-level mouse clicks now route through `mouseSelectRow` so the row selection bucket (which `getSelectedItems()` and the `.selected` class read) actually gets populated. - The default `.focused` row styling is now gated on **both** cursor-on-row AND grid keyboard focus, mirroring the pattern used by `ColumnSummaryCell`. The focus ring disappears when focus leaves the list. - `PositronListItemContext` exposes `cursor`, `listFocused`, and `selected` so caller-rendered items can implement custom focus/selection styling. - `.positron-list-row` switched from `outline` to a transparent border with `box-sizing: border-box`, so the focus ring color swap doesn't reflow content. ## Section row handling - `PositronList` already had `isRowSelectable` skipping section rows during keyboard navigation. This branch refines it further: - `moveCursorUp` now reveals the adjacent unselectable row above the cursor (just one — not a whole stack). - New `firstSelectableRowIndex` / `lastSelectableRowIndex` getters on `DataGridInstance` make Cmd+Home and Cmd+End land on the first/last *selectable* row, skipping section headers. ## Bug fixes in shift-extend selection Fixed five truthy-zero bugs in `extend{Row,Column}Selection{Up,Down,Left,Right}` where `if (!position)` short-circuited on the valid position `0`. Shift-arrow extension now works correctly: - shift-right from column 0 - shift-left when the first selected column is 0 - shift-up when the first selected row is 0 - shift-up from row 0 with no prior selection - shift-down when the first selected row is 0 ## Custom context menu Refinements stacking on prior work: - `CustomContextMenuSeparator` accepts an optional title. - Items support a `destructive` style and an `iconSrc` (e.g., language icons). - `checked` and `destructive` are now mutually exclusive at the type level. <p> <img width="185" height="227" alt="image" src="https://github.com/user-attachments/assets/2b52cabf-7f46-4609-8dcc-14bd60f5db49" /> </p> ## Data Connections - New `dataConnections.enabled` feature flag (hidden from settings UI; settable via `settings.json`, requires reload) gating the experimental Data Connections view container/view. - "No data connections" empty state updated to match the Variables panel. - Various small refinements in the panel. Tests: @:data-explorer ### Release Notes #### New Features - N/A #### Bug Fixes - N/A ### Validation Steps - [ ] Arrow keys move the PositronList cursor and selection together; shift-arrow extends selection - [ ] Clicking a list item selects it; shift-click extends; section headers ignore clicks - [ ] Focus ring shows only when the list has keyboard focus; disappears on blur - [ ] Cmd+Home / Cmd+End land on a selectable row when the first/last row is a section header - [ ] Main data grid and table summary keyboard behavior unchanged (spreadsheet mode preserved) - [ ] Shift-right/left/up/down from row 0 or column 0 now extends selection correctly - [ ] Setting `dataConnections.enabled: true` in `settings.json` shows the Data Connections view after reload - [ ] Context menu items render with destructive styling and icons where configured --------- Co-authored-by: Brice Stacey <bricestacey@gmail.com>
1 parent 27c03df commit 513b849

17 files changed

Lines changed: 418 additions & 154 deletions

File tree

src/vs/platform/positronActionBar/browser/positronDynamicActionBar.tsx

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,16 +30,14 @@ export const DEFAULT_ACTION_BAR_DROPDOWN_BUTTON_WIDTH = 36;
3030
export const DEFAULT_ACTION_BAR_SEPARATOR_WIDTH = 7;
3131

3232
/**
33-
* OverflowContextMenuItem interface.
33+
* OverflowContextMenuItem type.
3434
*/
35-
export interface OverflowContextMenuItem extends CustomContextMenuItemOptions {
36-
}
35+
export type OverflowContextMenuItem = CustomContextMenuItemOptions;
3736

3837
/**
39-
* OverflowContextMenuSubmenu interface.
38+
* OverflowContextMenuSubmenu type.
4039
*/
41-
export interface OverflowContextMenuSubmenu extends CustomContextMenuSubmenuOptions {
42-
}
40+
export type OverflowContextMenuSubmenu = CustomContextMenuSubmenuOptions;
4341

4442
/**
4543
* DynamicActionBarAction interface.

src/vs/workbench/browser/positronComponents/customContextMenu/customContextMenu.css

Lines changed: 29 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,13 @@
1818
background: var(--vscode-positronContextMenu-separatorBackground);
1919
}
2020

21+
.custom-context-menu-separator-title {
22+
padding: 2px 8px;
23+
font-size: 0.9em;
24+
text-transform: uppercase;
25+
color: var(--vscode-descriptionForeground);
26+
}
27+
2128
.custom-context-menu-item {
2229
width: 100%;
2330
height: 26px;
@@ -29,11 +36,15 @@
2936
align-content: center;
3037
background: transparent;
3138
color: var(--vscode-positronContextMenu-foreground);
32-
grid-template-columns: [icon] 22px [title] 1fr [shortcut] min-content [end];
39+
grid-template-columns: [icon] 24px [title] 1fr [shortcut] min-content [end];
3340
}
3441

3542
.custom-context-menu-item.checkable {
36-
grid-template-columns: [check] 22px [icon] 22px [title] 1fr [shortcut] min-content [end];
43+
grid-template-columns: [check] 22px [icon] 24px [title] 1fr [shortcut] min-content [end];
44+
}
45+
46+
.custom-context-menu-item.destructive {
47+
color: var(--vscode-errorForeground);
3748
}
3849

3950
.custom-context-menu-item:not(.disabled):hover {
@@ -42,6 +53,10 @@
4253
background: var(--vscode-positronContextMenu-hoverBackground);
4354
}
4455

56+
.custom-context-menu-item.destructive:not(.disabled):hover {
57+
color: var(--vscode-errorForeground);
58+
}
59+
4560
.custom-context-menu-item:focus {
4661
outline: none !important;
4762
}
@@ -65,8 +80,15 @@
6580
grid-column: icon / title;
6681
}
6782

68-
.custom-context-menu-item .icon.disabled {
69-
opacity: 50%;
83+
.custom-context-menu-item .icon.icon-image {
84+
width: 16px;
85+
height: 16px;
86+
align-self: center;
87+
object-fit: contain;
88+
}
89+
90+
.custom-context-menu-item .icon.destructive {
91+
color: var(--vscode-errorForeground);
7092
}
7193

7294
.custom-context-menu-item .title {
@@ -76,11 +98,7 @@
7698
white-space: nowrap;
7799
text-overflow: ellipsis;
78100
grid-column: title / shortcut;
79-
margin: 2px 0 2px 0;
80-
}
81-
82-
.custom-context-menu-item .title.disabled {
83-
opacity: 50%;
101+
margin: 2px 0;
84102
}
85103

86104
.custom-context-menu-item .shortcut {
@@ -98,7 +116,8 @@
98116
grid-column: shortcut / end;
99117
}
100118

101-
/* Make the submenu chevron indicator look disabled */
119+
.custom-context-menu-item .icon.disabled,
120+
.custom-context-menu-item .title.disabled,
102121
.custom-context-menu-item .submenu-indicator.disabled {
103122
opacity: 50%;
104123
}

src/vs/workbench/browser/positronComponents/customContextMenu/customContextMenu.tsx

Lines changed: 57 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -30,14 +30,18 @@ const SUBMENU_HOVER_DELAY = 300;
3030
export type CustomContextMenuEntry = CustomContextMenuItem | CustomContextMenuSeparator | CustomContextMenuSubmenu;
3131

3232
/**
33-
* CustomContextMenuSubmenuOptions interface.
33+
* Icon options. A submenu may specify either a codicon name (`icon`) or an image source
34+
* (`iconSrc`), but not both. Defined as a discriminated union so callers can't accidentally
35+
* set both fields.
3436
*/
35-
export interface CustomContextMenuSubmenuOptions {
36-
/**
37-
* Optional icon to display before the label.
38-
*/
39-
readonly icon?: string;
37+
type CustomContextMenuSubmenuIconOptions =
38+
| { readonly icon?: string; readonly iconSrc?: never }
39+
| { readonly icon?: never; readonly iconSrc?: string };
4040

41+
/**
42+
* CustomContextMenuSubmenuOptions type.
43+
*/
44+
export type CustomContextMenuSubmenuOptions = CustomContextMenuSubmenuIconOptions & {
4145
/**
4246
* The label text for the submenu item.
4347
*/
@@ -53,7 +57,7 @@ export interface CustomContextMenuSubmenuOptions {
5357
* is opened to ensure properties like checked state are up to date in the submenu when it opens.
5458
*/
5559
readonly entries: () => CustomContextMenuEntry[];
56-
}
60+
};
5761

5862
/**
5963
* CustomContextMenuSubmenu class.
@@ -217,15 +221,22 @@ const CustomContextMenuModalPopup = (props: CustomContextMenuModalPopupProps) =>
217221
*
218222
* @returns The rendered component.
219223
*/
220-
const MenuSeparator = () => {
224+
const MenuSeparator = ({ title }: { title?: string }) => {
221225
// Render.
222226
return (
223-
<div
224-
className='custom-context-menu-separator'
225-
role='separator'
226-
// When the mouse enters a menu separator, close the active submenu.
227-
onMouseEnter={closeActiveSubmenu}
228-
/>
227+
<>
228+
<div
229+
className='custom-context-menu-separator'
230+
role='separator'
231+
// When the mouse enters a menu separator, close the active submenu.
232+
onMouseEnter={closeActiveSubmenu}
233+
/>
234+
{title && (
235+
<div className='custom-context-menu-separator-title'>
236+
{title}
237+
</div>
238+
)}
239+
</>
229240
);
230241
};
231242

@@ -261,7 +272,8 @@ const CustomContextMenuModalPopup = (props: CustomContextMenuModalPopupProps) =>
261272
<Button
262273
className={positronClassNames(
263274
'custom-context-menu-item',
264-
{ 'checkable': options.checked !== undefined }
275+
{ 'checkable': options.checked !== undefined },
276+
{ 'destructive': options.destructive }
265277
)}
266278
disabled={options.disabled}
267279
// When the mouse enters a menu item, close the active submenu.
@@ -283,16 +295,29 @@ const CustomContextMenuModalPopup = (props: CustomContextMenuModalPopupProps) =>
283295
/>
284296
}
285297

286-
{options.icon &&
287-
<div
298+
{options.icon
299+
? <div
288300
className={positronClassNames(
289301
'icon',
290302
'codicon',
291303
`codicon-${options.icon}`,
292-
{ 'disabled': options.disabled }
304+
{ 'disabled': options.disabled },
305+
{ 'destructive': options.destructive }
293306
)}
294307
title={options.label}
295308
/>
309+
: options.iconSrc &&
310+
<img
311+
alt=''
312+
className={positronClassNames(
313+
'icon',
314+
'icon-image',
315+
{ 'disabled': options.disabled },
316+
{ 'destructive': options.destructive }
317+
)}
318+
src={options.iconSrc}
319+
title={options.label}
320+
/>
296321
}
297322

298323
<div
@@ -434,8 +459,8 @@ const CustomContextMenuModalPopup = (props: CustomContextMenuModalPopupProps) =>
434459
onMouseLeave={cancelHoverTimer}
435460
onPressed={openSubmenu}
436461
>
437-
{options.icon &&
438-
<div
462+
{options.icon
463+
? <div
439464
aria-hidden='true'
440465
className={positronClassNames(
441466
'icon',
@@ -444,6 +469,17 @@ const CustomContextMenuModalPopup = (props: CustomContextMenuModalPopupProps) =>
444469
{ 'disabled': options.disabled }
445470
)}
446471
/>
472+
: options.iconSrc &&
473+
<img
474+
alt=''
475+
aria-hidden='true'
476+
className={positronClassNames(
477+
'icon',
478+
'icon-image',
479+
{ 'disabled': options.disabled }
480+
)}
481+
src={options.iconSrc}
482+
/>
447483
}
448484

449485
<div
@@ -502,7 +538,7 @@ const CustomContextMenuModalPopup = (props: CustomContextMenuModalPopupProps) =>
502538
if (entry instanceof CustomContextMenuItem) {
503539
return <MenuItem key={index} {...entry.options} />;
504540
} else if (entry instanceof CustomContextMenuSeparator) {
505-
return <MenuSeparator key={index} />;
541+
return <MenuSeparator key={index} title={entry.title} />;
506542
} else if (entry instanceof CustomContextMenuSubmenu) {
507543
return <MenuSubmenuItem key={index} {...entry.options} />;
508544
} else {

src/vs/workbench/browser/positronComponents/customContextMenu/customContextMenuItem.ts

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,24 +6,41 @@
66
import { KeyboardModifiers } from '../../../../base/browser/ui/positronComponents/button/button.js';
77

88
/**
9-
* CustomContextMenuItemOptions interface.
9+
* Icon options. A menu item may specify either a codicon name (`icon`) or an image source
10+
* (`iconSrc`), but not both. Defined as a discriminated union so callers can't accidentally
11+
* set both fields.
1012
*/
11-
export interface CustomContextMenuItemOptions {
13+
type CustomContextMenuItemIconOptions =
14+
| { readonly icon?: string; readonly iconSrc?: never }
15+
| { readonly icon?: never; readonly iconSrc?: string };
16+
17+
/**
18+
* Behavior options. A menu item may either represent a toggleable state (`checked`) or a
19+
* destructive action (`destructive`), but not both. A "checked destructive" item has no
20+
* coherent meaning -- you don't toggle a delete on and off. Defined as a discriminated
21+
* union so callers can't accidentally set both fields.
22+
*/
23+
type CustomContextMenuItemBehaviorOptions =
24+
| { readonly checked?: boolean; readonly destructive?: never }
25+
| { readonly checked?: never; readonly destructive?: boolean };
26+
27+
/**
28+
* CustomContextMenuItemOptions type.
29+
*/
30+
export type CustomContextMenuItemOptions = CustomContextMenuItemIconOptions & CustomContextMenuItemBehaviorOptions & {
1231
readonly commandId?: string;
1332
/**
1433
* Called BEFORE the command executes. Similar to native ActionRunner's onWillRun.
1534
*/
1635
readonly onWillSelect?: () => void;
17-
readonly checked?: boolean;
18-
readonly icon?: string;
1936
readonly label: string;
2037
readonly disabled?: boolean;
2138
/**
2239
* Called AFTER the command executes (or immediately if no commandId).
2340
* Similar to native ActionRunner's onDidRun.
2441
*/
2542
readonly onSelected: (e: KeyboardModifiers) => void;
26-
}
43+
};
2744

2845
/**
2946
* CustomContextMenuItem class.

src/vs/workbench/browser/positronComponents/customContextMenu/customContextMenuSeparator.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,4 +7,10 @@
77
* CustomContextMenuSeparator class.
88
*/
99
export class CustomContextMenuSeparator {
10+
/**
11+
* Constructor.
12+
* @param title An optional title to render as a section header below the separator line.
13+
*/
14+
constructor(readonly title?: string) {
15+
}
1016
}

0 commit comments

Comments
 (0)