Improve Login to DevSandbox button workflow#2985
Improve Login to DevSandbox button workflow#2985mohitsuman merged 1 commit intoredhat-developer:mainfrom
Conversation
|
|
||
| static async validateLoginToken(userClusterUrl: string, userToken: string): Promise<boolean> { | ||
| try { | ||
| await Cluster.cli.executeTool(Command.odoLoginWithToken(userClusterUrl, userToken.trim()), undefined, true); |
There was a problem hiding this comment.
I'm not sure if there is a better way to check whether the token is valid or not
There was a problem hiding this comment.
I think the regular expression check should be enough. I think that logging in and out of the cluster isn't necessary, and might slow down the extension. If the contents of the clipboard match the regex, but the token isn't valid, then we could show a popup when they hit the "Login to DevSandbox" button
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #2985 +/- ##
==========================================
- Coverage 37.59% 37.44% -0.15%
==========================================
Files 62 62
Lines 3937 3955 +18
Branches 755 758 +3
==========================================
+ Hits 1480 1481 +1
- Misses 2457 2474 +17
☔ View full report in Codecov by Sentry. |
mohitsuman
left a comment
mohitsuman
left a comment
There was a problem hiding this comment.
Change around active status of the Login button.
JessicaJHee
force-pushed
the
2977-login-DevSandbox
branch
from
500bd9b to
2022a87
Compare
|
|
||
| static async validateLoginToken(userClusterUrl: string, userToken: string): Promise<boolean> { | ||
| try { | ||
| await Cluster.cli.executeTool(Command.odoLoginWithToken(userClusterUrl, userToken.trim()), undefined, true); |
There was a problem hiding this comment.
I think the regular expression check should be enough. I think that logging in and out of the cluster isn't necessary, and might slow down the extension. If the contents of the clipboard match the regex, but the token isn't valid, then we could show a popup when they hit the "Login to DevSandbox" button
JessicaJHee
force-pushed
the
2977-login-DevSandbox
branch
from
2022a87 to
a613a15
Compare
|
Updated:
|
| await new Promise(r => setTimeout(r, 500)); | ||
| const currentContent = await vscode.env.clipboard.readText(); | ||
| if (previousContent && previousContent !== currentContent) { | ||
| const oauthInfo = await sandboxAPI.getOauthServerInfo(signupStatus.apiEndpoint); |
There was a problem hiding this comment.
Since sending and receiving the response from the auth server takes some time, the user could copy the token in this time. That means the change in content won't be detected.
I ran into this. As a result, I can't click the button unless I already have the token in my clipboard when I open the cluster view.
JessicaJHee
force-pushed
the
2977-login-DevSandbox
branch
from
a613a15 to
249da31
Compare
|
One more quick comment (sorry it's so late, but hopefully it's easy to address). The tooltip for the login button slightly covers up the bottom of the button:
This is inconsistent with the rest of the buttons in this row, and makes it hard to click on the bottom part of the button. If this is too difficult to address, then don't worry about it. |
datho7561
left a comment
datho7561
left a comment
There was a problem hiding this comment.
Other than the issue I mentioned with the tooltip, this looks good. Feel free to merge. Thanks, Jessica!
Signed-off-by: Jessica He <jhe@redhat.com>
JessicaJHee
force-pushed
the
2977-login-DevSandbox
branch
from
249da31 to
80a9054
Compare
3 participants
Fixes #2977