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

redirect to desktop IDE on app domain #12082

Merged
merged 1 commit into from
Aug 15, 2022
Merged

redirect to desktop IDE on app domain #12082

merged 1 commit into from
Aug 15, 2022

Conversation

akosyakov
Copy link
Member

@akosyakov akosyakov commented Aug 12, 2022

Description

An idea to redirect always on parent app domain, i.e. gitpod.io, via iframe to suppress popup on each new workspace start.

Screenshot 2022-08-12 at 09 09 44

Important We need to consider a case if a user does not have yet app, but allowed pop-up always for some reason, i.e. for another IDE.

Related Issue(s)

Fixes #

How to test

  • Select VS Code Desktop in settings.
  • Start one workspace, click allow always in pop-up.
  • Start another workspace, check that pop-up does not come anymore. VS Code Desktop is opened automatically.

Release Notes

Enable allowing redirect to Desktop IDEs for all workspaces.

Documentation

Werft options:

  • /werft with-preview

@werft-gitpod-dev-com
Copy link

started the job as gitpod-build-ak-redirect-on-parent.1 because the annotations in the pull request description changed
(with .werft/ from main)

@loujaybee
Copy link
Member

Oh wow, tested, and works great. IMO this redirect works more as a user would expect, once they allow once for Gitpod, doesn't seem relevant to keep showing this for different workspaces. A couple thoughts:

  1. I cannot see an easy to get the prompt back (if I wanted to), without going to: code ~/Library/Application\ Support/Google/Chrome/Default/Preferences, and manually fiddling with the preferences, however that said we can easily document this anyway.

  2. On mac, this will "pull" the focus to the desktop where VS Code is already opened (if it is). This isn't really related to this issue though, and is something we can investigate separately.

@loujaybee
Copy link
Member

loujaybee commented Aug 12, 2022

We need to consider a case if a user does not have yet app - @akosyakov

Thought(non-blocking):

  1. If it's the users first experience, add an additional prompt asking the user to download, else dismiss (as in the NowTV example given in issue linked below).
  2. Ensure that we are catching and handling any redirect errors where the external redirect fails.

We already have an issue for JetBrains for this, we can make it for both JetBrains and VS Code...

In both cases we could provide direct download embeds to download the app:

Also related:

CC: @gtsiolis

@akosyakov
Copy link
Member Author

akosyakov commented Aug 12, 2022

Ensure that we are catching and handling any redirect errors where the external redirect fails.

yes, it is also my thought if we want to suppress the dialog then we should handle errors and show own dialog. But from other side we already have a message in grayish which starts with if you **don't** see a dialog.... So maybe not a blocker?

In both cases we could provide direct download embeds to download the app:

I'm still uncomfortable with that. Because without toolbox users cannot upgrade Gateway and it will cause an issue with each major release. We could insert direct links to toolbox downloadable instead? I think this can be done as another PR though.

@loujaybee
Copy link
Member

loujaybee commented Aug 12, 2022

I think this can be done as another PR though. - @akosyakov

So maybe not a blocker? - @akosyakov

Ah yes, sorry my general sentiment is: "we have some proposals already lined up to catch/handle the failure states for the user" so we should be good to proceed with this PR and look at those UX niceties after for both JB + VS Code 🚀

Agree with your thoughts / concerns / suggestions on the embeds, we can discuss in the issue directly 🙏

@akosyakov
Copy link
Member Author

akosyakov commented Aug 12, 2022

/hold

@akosyakov
Copy link
Member Author

We need to think how we can pull the same trick for copy/paste feature.

@akosyakov akosyakov requested a review from a team August 15, 2022 12:11
Base automatically changed from ak/open_desktop_link to main August 15, 2022 12:25
@akosyakov akosyakov removed the request for review from a team August 15, 2022 12:47
@akosyakov
Copy link
Member Author

/unhold

@roboquat roboquat merged commit 81c98f8 into main Aug 15, 2022
@roboquat roboquat deleted the ak/redirect_on_parent branch August 15, 2022 13:26
@roboquat roboquat added deployed: IDE IDE change is running in production deployed Change is completely running in production labels Aug 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deployed: IDE IDE change is running in production deployed Change is completely running in production release-note size/M team: IDE
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants