Skip to content

Conversation

@kettanaito
Copy link
Contributor

@kettanaito kettanaito commented Aug 22, 2022

CodeSandbox logoCodeSandbox logo  Open in CodeSandbox Web Editor | VS Code

What kind of change does this PR introduce?

This is a new feature, a follow-up to the VS Code Insiders support in CodeSandbox Projects.

What is the current behavior?

Currently, when going through the VS Code login flow from the VS Code Insiders, the callback URI is always hardcoded on the V1's side to be vscode://. This will open the regular VS Code app and not the insiders app that has initiated the login.

What is the new behavior?

  • Composes the callback URI dynamically based on the insiders query parameter present in the vscode/login URL;
  • Rewrites the hooks logic because you don't need to circle it through a useEffect to set the deepLink on the link.
  • Displays a warning notification if the user doesn't have VS Code Insiders installed in their system;
  • Displays an error notification if there's a different error while trying to open the vscode-insiders:// callback URI.

What steps did you take to test this? This is required before we can merge, make sure to test the flow you've updated.

  1. Deployed to staging.
  2. Tested manually on staging.

Screenshots

Screenshot 2022-08-23 at 13 17 16

Testing that clicking the button actually leads to Insiders if installed:

vscode-ins.mov

The auth token in the video is rightfully invalid because I'm testing this from the local dev environment.

Checklist

  • Documentation (not necessary; internal change).
  • Testing
  • Ready to be merged
  • Added myself to contributors table
  • Clicking on the "Open in Visual Studio Code Insiders" button doesn't trigger the notification when the insiders build is not installed. On load notification is displayed correctly.

"posthtml-parser": "^0.4.1",
"posthtml-render": "^1.1.0",
"prism-react-renderer": "^1.0.2",
"protocol-handlers": "^0.1.2",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using the same library we use in V2 to detect protocol handlers that don't have any registered apps in the system.

@codesandbox-ci
Copy link

codesandbox-ci bot commented Aug 22, 2022

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit a3defc1:

Sandbox Source
Notifications Test Configuration

@kettanaito kettanaito marked this pull request as ready for review August 22, 2022 10:04
@kettanaito
Copy link
Contributor Author

The protocol-handlers was published with a syntax that's not supported (private class properties declaration). I've changed it to es2020, and the build should pass after I update the dependency.

@lbogdan lbogdan temporarily deployed to pr6836 August 22, 2022 14:51 Inactive
@lbogdan
Copy link
Contributor

lbogdan commented Aug 22, 2022

Build for latest commit a3defc1 is at https://pr6836.build.csb.dev/s/new.

@kettanaito kettanaito force-pushed the feat/vscode-login-insiders branch from a415db9 to f3665ba Compare August 23, 2022 10:26
@lbogdan lbogdan temporarily deployed to pr6836 August 23, 2022 10:37 Inactive
autoWidth
href={deepLink}
style={{ fontSize: 16, height: 40, width: '100%', marginTop: '1rem' }}
onClick={openInVsCode}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Converting this link to a button because if we navigate to vscode-insiders:// from a link we cannot catch the exception that happens when the protocol handler is not registered. We use the openUrl from protocol-handlers to check for that, so we need an onClick.

@kettanaito kettanaito requested a review from CompuIves August 23, 2022 12:31
@kettanaito
Copy link
Contributor Author

Adding support for conditional sign in urls from the extension's side in https://github.com/codesandbox/codesandbox-applications/pull/1974.

@kettanaito kettanaito requested a review from gsimone August 24, 2022 11:06
@kettanaito kettanaito merged commit d8705ea into master Aug 24, 2022
@kettanaito kettanaito deleted the feat/vscode-login-insiders branch August 24, 2022 14:58
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.

4 participants