-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Auto logout after specific time #6558
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code looks good to me. We should get at least one QA at a longer time before merging.
I will re-review once test changes are made.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@chikeichan It seems the necessary changes to advanced-tab.container.js
have been removed from this PR. For instance, setAutoLogoutTimeLimit
is not defined anywhere
wow, let me check on this later today. Thanks for pointing this out |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good now
Couple Notes:
In the screen cap, timer is set to respond in seconds instead of minutes for the sake of time
react-idle-timer
is fairly actively maintained -> 20+ commits within last 6 months. I think the risk is trivial, but I am open to refactoring into our own idle-timer module.unit test
fix/update integration test