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

There is no any progress indication on Login/Switch context/Change pr… #3855

Conversation

vrubezhny
Copy link
Contributor

…oject #3849

Fixes: #3849

@vrubezhny vrubezhny marked this pull request as draft January 30, 2024 19:04
Copy link

codecov bot commented Jan 30, 2024

Codecov Report

Attention: Patch coverage is 23.52941% with 65 lines in your changes are missing coverage. Please review.

Project coverage is 44.82%. Comparing base (da60441) to head (4076940).
Report is 110 commits behind head on main.

Files Patch % Lines
src/openshift/cluster.ts 23.52% 65 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #3855       +/-   ##
===========================================
+ Coverage   32.37%   44.82%   +12.44%     
===========================================
  Files          85       85               
  Lines        6505     6742      +237     
  Branches     1349     1406       +57     
===========================================
+ Hits         2106     3022      +916     
+ Misses       4399     3720      -679     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@vrubezhny vrubezhny self-assigned this Jan 31, 2024
@vrubezhny vrubezhny force-pushed the fix-add-progress-to-login-logout-and-switch-context-project branch from 88fd10e to 4076940 Compare March 15, 2024 18:00
@vrubezhny vrubezhny force-pushed the fix-add-progress-to-login-logout-and-switch-context-project branch 3 times, most recently from 254564d to 1a5fef7 Compare April 4, 2024 17:09
@vrubezhny vrubezhny marked this pull request as ready for review April 5, 2024 13:11
@vrubezhny
Copy link
Contributor Author

A short demo of how the login/logout/switch context operations are notified in Status Bar:

Screencast.from.2024-03-15.18-43-46.webm

Copy link
Contributor

@datho7561 datho7561 left a comment

Choose a reason for hiding this comment

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

If I close the login wizard after I have input the username, then the status bar "Logging in..." remains on the status bar

src/openshift/cluster.ts Outdated Show resolved Hide resolved
src/openshift/cluster.ts Outdated Show resolved Hide resolved
src/openshift/cluster.ts Outdated Show resolved Hide resolved
src/openshift/cluster.ts Outdated Show resolved Hide resolved
src/openshift/cluster.ts Outdated Show resolved Hide resolved
src/openshift/cluster.ts Outdated Show resolved Hide resolved
src/openshift/cluster.ts Outdated Show resolved Hide resolved
src/openshift/cluster.ts Show resolved Hide resolved
src/openshift/cluster.ts Show resolved Hide resolved
src/openshift/cluster.ts Outdated Show resolved Hide resolved
@vrubezhny vrubezhny marked this pull request as draft April 12, 2024 21:58
@codecov-commenter
Copy link

codecov-commenter commented Apr 12, 2024

Codecov Report

Attention: Patch coverage is 32.03125% with 87 lines in your changes are missing coverage. Please review.

Project coverage is 45.03%. Comparing base (da60441) to head (e055d82).
Report is 177 commits behind head on main.

Files Patch % Lines
src/openshift/cluster.ts 30.57% 84 Missing ⚠️
src/util/inputValue.ts 50.00% 2 Missing ⚠️
src/oc/ocWrapper.ts 66.66% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #3855       +/-   ##
===========================================
+ Coverage   32.37%   45.03%   +12.65%     
===========================================
  Files          85       85               
  Lines        6505     6804      +299     
  Branches     1349     1428       +79     
===========================================
+ Hits         2106     3064      +958     
+ Misses       4399     3740      -659     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@vrubezhny vrubezhny force-pushed the fix-add-progress-to-login-logout-and-switch-context-project branch from 1a5fef7 to cc64c83 Compare April 17, 2024 18:53
@vrubezhny
Copy link
Contributor Author

vrubezhny commented Apr 18, 2024

If I close the login wizard after I have input the username, then the status bar "Logging in..." remains on the status bar

@datho7561 Could you please re-test your scenario using the updated PR? I can't reproduce the issue.

Also, please look at the comment regarding the use of Node's fetch here: #3855 (comment)

@vrubezhny vrubezhny requested a review from datho7561 April 18, 2024 01:42
@vrubezhny vrubezhny force-pushed the fix-add-progress-to-login-logout-and-switch-context-project branch from cc64c83 to de1a5f6 Compare April 18, 2024 11:02
@vrubezhny vrubezhny marked this pull request as ready for review April 18, 2024 12:02
@datho7561
Copy link
Contributor

datho7561 commented Apr 18, 2024

If I close the login wizard after I have input the username, then the status bar "Logging in..." remains on the status bar

I can still reproduce the bug if I close the wizard by pressing the Escape key, but if it's hard to get that case to work, I think the current behaviour is good.

datho7561
datho7561 previously approved these changes Apr 18, 2024
Copy link
Contributor

@datho7561 datho7561 left a comment

Choose a reason for hiding this comment

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

Found a typo in the documentation. If you want to fix it, go ahead, but I think it's good to merge as is.

src/openshift/cluster.ts Outdated Show resolved Hide resolved
@vrubezhny
Copy link
Contributor Author

If I close the login wizard after I have input the username, then the status bar "Logging in..." remains on the status bar

I can still reproduce the bug if I close the wizard by pressing the Escape key, but if it's hard to get that case to work, I think the current behaviour is good.

Thanks. Now I can reproduce the issue - it really leaves the notification displayed if I press ESC in username selection quick pick

@vrubezhny vrubezhny force-pushed the fix-add-progress-to-login-logout-and-switch-context-project branch from de1a5f6 to e055d82 Compare April 18, 2024 16:20
@vrubezhny
Copy link
Contributor Author

If I close the login wizard after I have input the username, then the status bar "Logging in..." remains on the status bar

I can still reproduce the bug if I close the wizard by pressing the Escape key, but if it's hard to get that case to work, I think the current behaviour is good.

Should be fixed now

@vrubezhny vrubezhny merged commit 0077288 into redhat-developer:main Apr 23, 2024
4 checks passed
@vrubezhny
Copy link
Contributor Author

The latest screencast of login/logout/switch contexts and repeated login while the previous login operation isn't finished (cancelling the previous operation):

Screencast.from.2024-04-23.14-51-47.webm

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.

There is no any progress indication on Login/Switch context/Change project
3 participants