Skip to content

[jb] enable connect button only when JetBrains Client not activated (cont.) #10216

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

Closed
wants to merge 1 commit into from

Conversation

yaohui-wyh
Copy link
Contributor

@yaohui-wyh yaohui-wyh commented May 24, 2022

Description

A cleanup PR for #10177

Related Issue(s)

Fixes #

How to test

Build & Install the plugin following
https://github.com/gitpod-io/gitpod/blob/main/components/ide/jetbrains/gateway-plugin/README.md#L8

Release Notes

NONE

Documentation

  • /werft no-preview=true

@yaohui-wyh
Copy link
Contributor Author

PTAL @akosyakov @felladrin

@akosyakov akosyakov requested a review from felladrin May 24, 2022 11:49
@akosyakov
Copy link
Member

akosyakov commented May 25, 2022

/werft run

👍 started the job as gitpod-build-jb-gw-fork.3
(with .werft/ from main)

@akosyakov
Copy link
Member

akosyakov commented May 26, 2022

/werft run

👍 started the job as gitpod-build-jb-gw-fork.5
(with .werft/ from main)

@akosyakov
Copy link
Member

akosyakov commented May 26, 2022

I wonder is it really a solution? You can still get with multiple connection windows while workspace is starting or stopping?

I still was able to cause such conditions as from browser as from GW if i click several times while workspace is starting. I think the proper fix is to ensure that there is only connection at the time, not a client.

@felladrin Could you revert the previous PR from main please? 🙏

@felladrin
Copy link
Contributor

I wonder is it really a solution? You can still get with multiple connection windows while workspace is starting or stopping?

I still was able to cause such conditions as from browser as from GW if i click several times while workspace is starting.

I've created the Revert PR: #10279

But couldn't we disable the button while the state of the workspace is "starting" or "stopping" to solve this case?

I think the proper fix is to ensure that there is only connection at the time, not a client.

But checking for a connection, wouldn't also prevent user from connecting from VS Code Web + JetBrains simultaneously? (I do it some times, when working with two different folders with different programming language)

@yaohui-wyh
Copy link
Contributor Author

yaohui-wyh commented May 26, 2022

I wonder is it really a solution? You can still get with multiple connection windows while workspace is starting or stopping?
I still was able to cause such conditions as from browser as from GW if i click several times while workspace is starting.

Yeah, this PR (together with PR #10177) can only prevent the user from clicking the "Connect" button (unintentionally) when JBClient is alive, which is indeed not a perfect solution (not able to cover the transitioning states such as starting & stopping). Sorry for not discussing a solution before-head, feel free to close the PR if you reach a conclusion. ❤️

@akosyakov
Copy link
Member

We could for instance try to fail if one is calling a connection provider but we already handling such URL. Ideally we should bring to focus client or connection window in such case. Maybe we at least have a control over focus for the connection window?

@loujaybee loujaybee requested a review from a team May 30, 2022 11:47
@felladrin
Copy link
Contributor

Maybe we at least have a control over focus for the connection window?

In an internal discussion, the Jetbrains team confirmed there's currently no API for controlling the focus of the Thin Client or the Connection Window from the Gateway but agreed it would be something to have there.

I'll file an issue for them and post the link here later for us to keep an eye on it.

@yaohui-wyh, is it ok for you if we close this PR? Or would you like to bring the changes from PR #10177 into this PR and try another approach for covering the case when the workspace is starting or stopping?

@yaohui-wyh
Copy link
Contributor Author

yaohui-wyh commented May 31, 2022

is it ok for you if we close this PR?

Yeah I agree that a better approach should be activating the running Thin Client, and since you have discussed it with the JetBrains Team, I think it's better to wait until such API is provided and do the enhancement then.
Thanks for all the suggestions @akosyakov @felladrin during the prev PR #10177 and this one. And again, sorry for the opinionated changes and for costing a lot of time for @felladrin back and forth. ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants