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

feat: close session on status bar #573

Merged
merged 1 commit into from
Nov 30, 2023

Conversation

scnwwu
Copy link
Member

@scnwwu scnwwu commented Oct 18, 2023

Summary
Resolve #119

  • Remove the disconnect button from the editor toolbar (next to the run button), as it's not specific to a file.
  • Change the icon of the active profile status bar item when a compute session established to provide visual indication.
  • Add Close Session button on the tooltip of the active profile status bar item when a compute session established.
  • Add SAS Profile title to the tooltip of the active profile status bar item
  • (Engineering) Extract status bar related code to a separate file

Testing
Create and close sessions to see the changes.

@scnwwu
Copy link
Member Author

scnwwu commented Oct 18, 2023

@clangsmith mentioned an idea to also have a Switch Profile button on the tooltip of the active profile status bar item. So that if user doesn't know that clicking the status bar item can switch profile, they may find it on the tooltip.
I played a bit with this idea but found I'm not a fan of it.
I worry if user first see the tooltip may think it's the only way or our expected way to switch profile. But it's not a good experience to have to hover on the item and wait for the tooltip to popup and move mouse carefully on to a text link, compared to simply click the item.
Every status bar item is clickable in VS Code. It's common. I think we should guide user to take this better approach.

@scnwwu scnwwu force-pushed the p-scnwwu-disconnect branch 2 times, most recently from 1e154a6 to 84860af Compare October 18, 2023 08:38
@scnwwu scnwwu requested a review from scottdover October 18, 2023 08:41
@scnwwu scnwwu added this to the 1.6.0 milestone Oct 27, 2023
Copy link
Contributor

@scottdover scottdover left a comment

Choose a reason for hiding this comment

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

This looks good to me, assuming things are okay with @clangsmith ui-wise. Sorry this one fell of my radar for so long

@Zhirong2022 Zhirong2022 added testing Test the pull requests testing-complete Complete the pull requests testing and removed verification-needed testing Test the pull requests labels Nov 30, 2023
@Zhirong2022
Copy link
Collaborator

The feature works as expected.

@scnwwu scnwwu force-pushed the p-scnwwu-disconnect branch from 84860af to 0417822 Compare November 30, 2023 08:59
@scnwwu scnwwu merged commit 6fb5236 into sassoftware:main Nov 30, 2023
1 check passed
@scnwwu scnwwu deleted the p-scnwwu-disconnect branch November 30, 2023 09:02
@Zhirong2022 Zhirong2022 added verified and removed testing-complete Complete the pull requests testing labels Dec 13, 2023
@Zhirong2022 Zhirong2022 added ready for release Code pushed, but not released in VS code marketplace yet and removed verified labels Jan 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready for release Code pushed, but not released in VS code marketplace yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add a visual indication whether a connection to a compute server exists or not
3 participants