Feature/multiple sessions tab view #6373

dhruvisompura · 2025-02-15T00:29:23Z

Description

Addresses #6160

This change introduces the new console tab list view in the console pane behind the positron.mulitpleConsoleSessions feature flag. The console tab list follows the tab list pattern for accessibility: https://www.w3.org/WAI/ARIA/apg/patterns/tabs/examples/tabs-automatic/

This issue does not address #6199 which is related since the console tab list can be resized. The console tab list resizing behavior can definitely use some improvement but can be tackled as polish.

Release Notes

New Features

N/A

Bug Fixes

N/A

QA Notes

Please see testing notes under #6160

Screenshots

Resize Console Pane

Screen.Recording.2025-02-18.at.8.31.34.PM.mov

Switch Sessions

Screen.Recording.2025-02-14.at.4.21.01.PM.mov

github-actions · 2025-02-15T00:29:39Z

E2E Tests 🚀
This PR will run tests tagged with: @:critical

^readme ^{valid tags}

dhruvisompura · 2025-02-15T00:31:38Z

src/vs/workbench/contrib/positronConsole/browser/components/consoleCore.css

+	width: 100%;
+}
+
+.positron-console .console-tab-list {


The flex properties should only be applied when the feature flag is on. This css class is conditionally applied if the feature flag is on.

dhruvisompura · 2025-02-15T00:33:10Z

src/vs/workbench/contrib/positronConsole/browser/components/actionBar.tsx

-							</div>
-						}
-					</ActionBarRegion>
+					{!multiSessionsEnabled &&


We no longer need to show this menu button since the console tab list does the same thing.

dhruvisompura · 2025-02-15T00:37:38Z

src/vs/workbench/contrib/positronConsole/browser/components/consoleCore.tsx

@@ -46,6 +61,37 @@ export const ConsoleCore = (props: ConsoleCoreProps) => {
 		return () => disposables.dispose();
 	});

+	// Console Width Effect
+	useEffect(() => {


This useEffect's only purpose is to set the initial width for the console pane/console tab list as well as handle resizing the console pane/console tab list when the parent container width changes.

The tricky bit of additional work that needs to be handled here is gracefully resizing the console pane and console tab list when the overall width of the container changes because a user increased the width of the secondary side bar (where the variables/plots panes live).

I had some gnarlier logic in this useEffect to try and resolve that but couldn't get it working in all scenarios. I'm keeping this useEffect simple for now and am tracking this as polish work in the epic that can be revisited after the core functionality is in.

@softwarenerd This may be a great task to pair on as follow up since you've dealt with these issues before!

dhruvisompura · 2025-02-15T00:46:51Z

src/vs/workbench/contrib/positronConsole/browser/components/consoleInstanceState.tsx

+	readonly positronConsoleInstance: IPositronConsoleInstance;
+}
+
+export const ConsoleInstanceState = ({ positronConsoleInstance }: ConsoleInstanceStateProps) => {


This component just renders the icon that maps to the state the session is in. Instead of relying on the session's runtime state value, I am using the state value from the console instance we create. The console instance's state is almost a 1-to-1 map and is probably more relevant to the user than just the session state.

I can imagine if the console is busy and blocks interaction for some reason, we may want to indicate that with this icon.

That being said, if this seems wrong and we think that we should be showing the runtime session state, I can change this pretty easily!

dhruvisompura · 2025-02-15T00:48:58Z

src/vs/workbench/contrib/positronConsole/browser/components/consoleTabList.tsx

+						<img
+							className='icon'
+							src={`data:image/svg+xml;base64,${positronConsoleInstance.session.runtimeMetadata.base64EncodedIconSvg}`}
+						/>


The mockup has us rendering a grayscale version of the icon but I didn't feel inclined to make an icon mapping to languages when we have access to the runtime language icon.

midleman · 2025-02-18T12:44:17Z

I've started adding a test to cover much of this new functionality. However, I've run into a couple issues:

The feature flag doesn't appear on web (this is reproducible in CI as well)
I’m having intermittent issues with Playwright when trying to click on the session tab in the list. It seems like the tab’s hitbox is overlapping with the Debug Console button, causing the inconsistent behavior. In the attached screenshot, you can see a misalignment. Playwright, by default, clicks the center of the locator - marked by the red dot in the image - but the center point is off, which could explain why the click fails. It seems likely that the overlapping hitboxes are causing Playwright to register the wrong element during the interaction.
I was hoping to add a test case around toggling the runtime in variables pane and ensure proper behavior in console, however, surprise, surprise, it's a native menu! So this could only be tested on web.

softwarenerd · 2025-02-18T19:36:16Z

src/vs/workbench/common/theme.ts

+	hcDark: '#2eb77c',
+	hcLight: '#2eb77c'
+}, localize('positronConsole.stateIconIdle', "Positron console idle state icon color."));
+


I'm a little worried about hard-coded colors when it comes to strange themes that a user might use.

I completely agree! We will probably need to change these colors and do something more reasonable but for now I figured this would be okay. I have added a note in my clean up list on the epic to revisit this.

I am open to changing this but I couldn't find any existing vs code vars to lean on 😢

samclark2015

Overall looks good! Tested and works flawlessly with my changes to the interpreter dropdown.

samclark2015 · 2025-02-19T15:00:11Z

src/vs/workbench/contrib/positronConsole/browser/components/consoleCore.tsx

+
+		// Initialize the width for the console pane and console tab list if it hasn't been
+		if (consoleWidth === 0) {
+			setConsoleTabListWidth(MAXIMUM_CONSOLE_TAB_LIST_WIDTH)


The tab pane feels too wide, and the console too squished. Is 2/5 too much for smaller displays? Maybe we need a width breakpoint - e.g. 2/5 when width > 1024, and 1/5 otherwise?

I am using 2/5ths as the default to match the look and feel of the Terminal tab list!

We could make the default tab list size smaller by default, maybe 1/5 if we feel strongly about it?

I'm not inclined to add breakpoints to Positron since all of the panes and sidebars can be resized by the user - the value of adding breakpoints is pretty low with resizing as an option.

I think @softwarenerd mentioned that we could store the window size and pane/sidebar sizes the user has set so that Positron loads up with those layout configurations. That would be the way to go, but we haven't added that yet.

dhruvisompura commented Feb 15, 2025

View reviewed changes

dhruvisompura added 12 commits February 14, 2025 16:49

Add missing semicolon

3154fb8

Allow console tab list to be resized

1319b8b

Lint file

275dd98

Display language icon in tab list

a628b04

Display icon for console state

042d527

Rely on console instance state instead of session state

d5d0442

Move console tab list behind FF

053362a

Hide console instance menu button when FF is on

a40adf6

Recalculate console width when container width changes

9d49627

Prevent tab list from disappearing for smallest screen size

79e33fb

Display directory behind FF

16b728f

Add console state icon colors to theme

91933a8

dhruvisompura force-pushed the feature/multiple-sessions-tab-view branch from 2663aef to 91933a8 Compare February 15, 2025 01:16

Cleanup useffect dep list

2a74d6d

dhruvisompura requested review from softwarenerd and samclark2015 February 15, 2025 01:18

dhruvisompura added 7 commits February 18, 2025 09:19

Fix typo in component name

2d53e29

Vertically center console tab contents

152827a

Fix tab focus

b301b57

Move feature flag into console settings

5d1f7bb

Add console feature tag

6daa3d0

Allow console child panes to resize smoothly

78f1689

Truncate console tab text and increase minimum tab width

82aa178

dhruvisompura added 2 commits February 18, 2025 19:31

Fix resizing bugs in console core

f04c4d9

Add data-testid to console tabs for CI tests

ed6025f

softwarenerd approved these changes Feb 19, 2025

View reviewed changes

samclark2015 reviewed Feb 19, 2025

View reviewed changes

dhruvisompura merged commit 2002887 into main Feb 19, 2025
9 checks passed

dhruvisompura deleted the feature/multiple-sessions-tab-view branch February 19, 2025 18:58

github-actions bot locked and limited conversation to collaborators Feb 19, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/multiple sessions tab view #6373

Feature/multiple sessions tab view #6373

dhruvisompura commented Feb 15, 2025 •

edited

Loading

github-actions bot commented Feb 15, 2025 •

edited

Loading

dhruvisompura Feb 15, 2025

dhruvisompura Feb 15, 2025

dhruvisompura Feb 15, 2025 •

edited

Loading

dhruvisompura Feb 15, 2025

dhruvisompura Feb 15, 2025

midleman commented Feb 18, 2025

softwarenerd Feb 18, 2025

dhruvisompura Feb 19, 2025

samclark2015 left a comment

samclark2015 Feb 19, 2025

dhruvisompura Feb 19, 2025

Feature/multiple sessions tab view #6373

Feature/multiple sessions tab view #6373

Conversation

dhruvisompura commented Feb 15, 2025 • edited Loading

Description

Release Notes

New Features

Bug Fixes

QA Notes

Screenshots

github-actions bot commented Feb 15, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dhruvisompura Feb 15, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

midleman commented Feb 18, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samclark2015 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dhruvisompura commented Feb 15, 2025 •

edited

Loading

github-actions bot commented Feb 15, 2025 •

edited

Loading

dhruvisompura Feb 15, 2025 •

edited

Loading