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

Idle Modal: Basic Implementation #8726

Closed
wants to merge 13 commits into from

Conversation

scures
Copy link
Contributor

@scures scures commented Apr 24, 2023

NOTE: This feature was split in 2 parts; while waiting for feedback I made a branch from this one and start working on the second part of it #8775 which will glue everything together + fixed/refactored most of the code in this initial PR; The review/testing focus should go into #8775

Summary

Fixes #8321

Technical notes summary

Added a new <Inactivity /> component

Areas or cases that should be tested

  • By default the component is disabled (this will change once we take care of the part 2 of the feature), to test it in /shell/layouts/default.vue#L770 change it to enable like this<Inactivity enabled />
  • Inside the /shell/components/Inactivity.vue they're some props you can play around like after how much time the initial modal will show and how much time does the user have until we unsubscribe from the WS.
  • While the timer is going on, you can resume the activity by clicking the Resume Session button and it will reset the inactive timer. Moving the mouse, pressing keys etc.. should keep reseting the timer, the moment you stop those actions, it will start counting down and after showModalAfter it will show the countdown.
  • If the timer reaches 0, we unsubscribe from sockets (having the devtool open you'll see 3 main actions for rancher, manager and, cluster

Screenshot/Video

image
image

@scures scures force-pushed the feat/inactivity-modal branch from 581a3de to ff7ac20 Compare April 26, 2023 21:01
@nwmac nwmac requested a review from richard-cox April 27, 2023 08:20
@richard-cox
Copy link
Member

@scures There's some test failures, can you take a look? Is it ready to come out of draft as well?

@scures scures marked this pull request as ready for review April 27, 2023 08:47
@scures
Copy link
Contributor Author

scures commented Apr 27, 2023

@richard-cox those tests look odd, I'll check them, it might be related with the comment of updating to 2.7 (which I already rebased).
It should be ready to review, yes. Removing the Draft part

@cnotv
Copy link
Member

cnotv commented Apr 27, 2023

You have 1 typo in the description, it's not Interactivity but Inactivity 😅

@cnotv
Copy link
Member

cnotv commented Apr 28, 2023

Copy link
Member

@cnotv cnotv 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 encountered 3 issues while testing the functionality:

  • The homepage has no inactivity, beside we have the component in the layout
  • In the first occurrence only, the countdown does start
  • After refreshing the page the whole logic stops working
Kapture.2023-04-28.at.11.52.45.mp4

Beside this, I wonder if we should not expose the inactivity status to Vuex, as other logic and components may profit from this. For instance we may avoid to use this component to start/stop subscription at a certain point and maybe wrapping the whole in a single action.

PS: There's some concerns related to the modal which have been added in #8207 as we do not want to tackle them here.

@scures scures requested a review from richard-cox April 28, 2023 10:26
@scures scures added the QA/None label May 3, 2023
@scures
Copy link
Contributor Author

scures commented May 3, 2023

This was split in 2 parts, while waiting for feedback I made a branch from this one and start working on the second part of it #8775 which will glue everything together + fixed/refactored most of the code in this initial PR; The review/testing focus should go into #8775

@scures scures requested a review from cnotv May 4, 2023 07:22
@richard-cox
Copy link
Member

Given #8775, can this one now be closed?

@scures
Copy link
Contributor Author

scures commented May 4, 2023

@richard-cox I wasn't sure if we want to keep the 2 PR's since we have the two issues. But if linking the two issues to 1 PR it's fine, lets do it =)

@cnotv
Copy link
Member

cnotv commented May 4, 2023

@scures You have to add the extra fix to the other PR and close this with a comment.
Actually that's true, it's better close than merge at this point 😅

@scures
Copy link
Contributor Author

scures commented May 5, 2023

Closing this PR for its continuity in #8775

@scures scures closed this May 5, 2023
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.

Idle Modal: Basic Implementation
3 participants