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

Switch to existing tab with the same workspace #1111

Merged
merged 6 commits into from
May 23, 2024
Merged

Conversation

akurinnoy
Copy link
Contributor

What does this PR do?

When opening a workspace, the dashboard looks for an already open tab with the same workspace. If a tab exists, the dashboard focuses it without creating a new tab.

What issues does this PR fix or reference?

resolves eclipse-che/che#22933

Is it tested? How?

  1. Create a new workspace, or open an existing one. A new browser tab should be created.
  2. Go back to the dashboard tab, and
    • click the same workspace in the Recent Workspaces section, or
    • click "Open" on the same workspace in the workspaces list, or
    • open the workspace actions menu for the same workspace, and click the "Open" item
  3. The existing workspace tab should be focused.

Signed-off-by: Oleksii Kurinnyi <okurinny@redhat.com>
Signed-off-by: Oleksii Kurinnyi <okurinny@redhat.com>
@che-bot
Copy link
Contributor

che-bot commented May 14, 2024

Click here to review and test in web IDE: Contribute

Copy link

Docker image build succeeded: quay.io/eclipse/che-dashboard:pr-1111

kubectl patch command
kubectl patch -n eclipse-che "checluster/eclipse-che" --type=json -p="[{"op": "replace", "path": "/spec/components/dashboard/deployment", "value": {containers: [{image: "quay.io/eclipse/che-dashboard:pr-1111", name: che-dashboard}]}}]"

@akurinnoy
Copy link
Contributor Author

/retest

@ibuziuk ibuziuk requested a review from dmytro-ndp May 15, 2024 13:31
Copy link
Contributor

@olexii4 olexii4 left a comment

Choose a reason for hiding this comment

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

I have tested these changes with Minikube. Everything looks like it was expected.

@openshift-ci openshift-ci bot removed the lgtm label May 15, 2024
Copy link

Docker image build succeeded: quay.io/eclipse/che-dashboard:pr-1111

kubectl patch command
kubectl patch -n eclipse-che "checluster/eclipse-che" --type=json -p="[{"op": "replace", "path": "/spec/components/dashboard/deployment", "value": {containers: [{image: "quay.io/eclipse/che-dashboard:pr-1111", name: che-dashboard}]}}]"

@dmytro-ndp
Copy link
Contributor

dmytro-ndp commented May 15, 2024

@akurinnoy : I have followed test scenario from description on Eclipse Che Next with quay.io/eclipse/che-dashboard:pr-1111 and failed on step 2:

  1. Go back to the dashboard tab, and
  • click the same workspace in the Recent Workspaces section, or
  • click "Open" on the same workspace in the workspaces list, or
  • open the workspace actions menu for the same workspace, and click the "Open" item

There was no new workspace in the Workspaces list, of RECENT WORKSPACES section of left panel after the new Empty workspace has started:
Screenshot from 2024-05-15 19-11-31

New running workspace had been displayed after the refresh of browser tab, but existing workspace tab wasn't focused.
Dashboard was opening new browser tab with start workspace page instead.

Note: it took some time between step 1 and 2, long enough to have user oauth session expired.
So, I had to log into User Dashboard again after the refresh of Dashboard tab.

Here the screencast of test case which is using logout to reproduce user oauth session expiration:
screencast-bpconcjcammlapcogcnnelfmaeghhagj-2024.05.15-21_10_27.webm

  1. Create a new workspace, or open an existing one. A new browser tab should be created.
  2. Go back to the dashboard tab, re-login, and
  • click the same workspace in the Recent Workspaces section, or
  • click "Open" on the same workspace in the workspaces list, or
  • open the workspace actions menu for the same workspace, and click the "Open" item

There were doubled workspace tab, which switched each other unexpectedly, and workspace start page showed error:

Failed to open the workspace
The workspace has not received an IDE URL in the last 20 seconds. Try to re-open the workspace.

Screenshot from 2024-05-15 21-24-57

Could you please take a look?

Copy link

Docker image build succeeded: quay.io/eclipse/che-dashboard:pr-1111

kubectl patch command
kubectl patch -n eclipse-che "checluster/eclipse-che" --type=json -p="[{"op": "replace", "path": "/spec/components/dashboard/deployment", "value": {containers: [{image: "quay.io/eclipse/che-dashboard:pr-1111", name: che-dashboard}]}}]"

@akurinnoy
Copy link
Contributor Author

@dmytro-ndp can you please test this PR one more time?

Copy link

codecov bot commented May 16, 2024

Codecov Report

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

Project coverage is 89.49%. Comparing base (7c64a6c) to head (743f13f).
Report is 2 commits behind head on main.

Files Patch % Lines
...ashboard-frontend/src/services/tabManager/index.ts 92.77% 6 Missing ⚠️
...dashboard-frontend/src/Layout/Navigation/index.tsx 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1111      +/-   ##
==========================================
- Coverage   89.63%   89.49%   -0.14%     
==========================================
  Files         407      408       +1     
  Lines       41480    41606     +126     
  Branches     2773     2769       -4     
==========================================
+ Hits        37180    37235      +55     
- Misses       4272     4346      +74     
+ Partials       28       25       -3     

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

@dmytro-ndp
Copy link
Contributor

dmytro-ndp commented May 17, 2024

@akurinnoy: thank you for the fixing relogin case.
Now Dashboard switches user to existing workspace tab, but redirecting to start workspace page instead of continue working in the same Editor window after the re-login. Not sure if it's expected behavior in case the user has uncommitted changes in the workspace, and so could lose them.
Screencast: relogin-2024.05.17-19_47_33.webm

I have also identified 2 more test cases:

Case "Open starting workspace again"

  1. Start new workspace creation in new tab.
  2. Go back to the dashboard tab before workspace is started and Editor is opened, and
  • click the same workspace in the Recent Workspaces section, or
  • click "Open" on the same workspace in the workspaces list, or
  • open the workspace actions menu for the same workspace, and click the "Open" item
  1. The existing workspace tab should be focused. Dashboard created new tab
    Screencast:
    create-several-workspaces-at-a-time-2024.05.17-19_45_07.webm

Case "Workspace with IntellijIDEA Community Editor"

  1. Create a new workspace with IntellijIDEA Community Editor.
    Go back to the dashboard tab, and
  • click the same workspace in the Recent Workspaces section, or
  • click "Open" on the same workspace in the workspaces list, or
  • open the workspace actions menu for the same workspace, and click the "Open" item
  1. The existing workspace tab should be focused. Dashboard was creating new tab first few seconds.
    Screencast:
    intellijIDEA-2024.05.17-20_07_03.webm

I have also noticed https://issues.redhat.com/browse/CRW-6287 error:
Screenshot from 2024-05-17 19-37-25

Copy link

Docker image build succeeded: quay.io/eclipse/che-dashboard:pr-1111

kubectl patch command
kubectl patch -n eclipse-che "checluster/eclipse-che" --type=json -p="[{"op": "replace", "path": "/spec/components/dashboard/deployment", "value": {containers: [{image: "quay.io/eclipse/che-dashboard:pr-1111", name: che-dashboard}]}}]"

@akurinnoy
Copy link
Contributor Author

Hi @dmytro-ndp ,

Now Dashboard switches user to existing workspace tab, but redirecting to start workspace page instead of continue working in the same Editor window after the re-login. Not sure if it's expected behavior in case the user has uncommitted changes in the workspace, and so could lose them.

This has been the case for quite some time, and this PR doesn't change that behavior.
During the re-login, the Dashboard page loses references to the previously opened tabs/windows and cannot set focus on them. In this case, the dashboard can only reuse the existing workspace tab (found by the window context string) to open the necessary workspace in it.

Case "Open starting workspace again"

Case "Workspace with IntellijIDEA Community Editor"

The latest fixup commit should fix these cases.

@dmytro-ndp
Copy link
Contributor

dmytro-ndp commented May 22, 2024

@akurinnoy : thanks for the addressing review notices!

Case "Open starting workspace again"

Case "Workspace with IntellijIDEA Community Editor"

The latest fixup commit should fix these cases.

I can confirm that those cases are working as expected in the fresh User Dashboard deployed from this PR (quay.io/eclipse/che-dashboard@sha256:b6e591b610156484f1eb6195293c575c1d6f2487198efcadad3f93aa7598a229).
Screencast: screencast-bpconcjcammlapcogcnnelfmaeghhagj-2024.05.22-18_37_12.webm

Good job!

I have also noticed that it was necessary to click several times on the workspace name in the left panel of the User Dashboard to redirect to the new tab, which was a bit annoying. However, I am not sure if this issue is specific to this PR because I encountered similar behavior on DevSandbox.

@openshift-ci openshift-ci bot added the lgtm label May 22, 2024
Copy link

openshift-ci bot commented May 22, 2024

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: akurinnoy, dmytro-ndp, ibuziuk, olexii4

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@akurinnoy akurinnoy merged commit 9f59c7a into main May 23, 2024
17 of 18 checks passed
@akurinnoy akurinnoy deleted the open-existing-tab branch May 23, 2024 06:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dashboard should switch to the tab with already running workspace
5 participants