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

Connections pane disconnected state #2810

Merged
merged 11 commits into from
Apr 30, 2024

Conversation

dfalbel
Copy link
Contributor

@dfalbel dfalbel commented Apr 19, 2024

Intent

Adds a disconnected state for the connections pane, with some UI for users to reconnect a closed connection.
Addresses #2739
Also addresses #2756
We are also addressing: #2792

Approach

  • Adds a new connectionTreeItem class that is used for closed connections
  • Store connections metadata in the project state using vscode's API for storing state
  • Various UI elements to re-connect, discard connections, copy connection code, etc.

QA Notes

Add additional information for QA on how to validate the change, paying special attention to the level of risk, adjacent areas that could be affected by the change, and any important contextual information not present in the linked issues.

Screen.Recording.2024-04-22.at.17.03.50.mov

@dfalbel dfalbel requested a review from jmcphers April 22, 2024 20:06
@dfalbel dfalbel marked this pull request as ready for review April 22, 2024 20:13
@dfalbel dfalbel requested a review from isabelizimm April 22, 2024 20:37
@dfalbel dfalbel force-pushed the feature/connections-disconnected-state branch 2 times, most recently from 4dd97bf to 564d6b3 Compare April 24, 2024 16:34
@dfalbel
Copy link
Contributor Author

dfalbel commented Apr 24, 2024

Tests are currently failing due to actions/runner-images#9770

@dfalbel dfalbel requested a review from isabelizimm April 24, 2024 23:52
@dfalbel dfalbel force-pushed the feature/connections-disconnected-state branch from 564d6b3 to d2d4d7b Compare April 29, 2024 15:49
@dfalbel dfalbel requested review from jmcphers and removed request for jmcphers April 29, 2024 17:07
extensions/positron-connections/src/connection.ts Outdated Show resolved Hide resolved
removeFromHistory(item: DisconnectedConnectionItem) {
this.context.workspaceState.update(item.name, undefined);
this._connections = this._connections.filter((connection) => {
return connection.name !== item.name;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it safe to match on name here? (seems like we could have two connections with the same name but different ids)

Copy link
Contributor

@isabelizimm isabelizimm left a comment

Choose a reason for hiding this comment

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

looks good for both types of sqllite connections 👍

@dfalbel dfalbel merged commit 1292d82 into main Apr 30, 2024
24 checks passed
@dfalbel dfalbel deleted the feature/connections-disconnected-state branch April 30, 2024 20:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants