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

Clear remember me from storage on submit and fix script path #304

Merged
merged 2 commits into from
Feb 2, 2025

Conversation

hobbitronics
Copy link
Contributor

@hobbitronics hobbitronics commented Jan 31, 2025

IDP-108 Keep "Remember me" unchecked if the user unchecks it (at least during a single login)

@hobbitronics hobbitronics requested a review from a team as a code owner January 31, 2025 09:22
@hobbitronics hobbitronics requested review from briskt, forevermatt, mtompset and jason-jackson and removed request for a team January 31, 2025 09:22
@hobbitronics hobbitronics merged commit 69d751a into main Feb 2, 2025
3 checks passed
@hobbitronics hobbitronics deleted the clear-remember-me-from-storage branch February 2, 2025 23:37
const form = document.querySelector('form');
if (form) {
form.addEventListener('submit', function() {
localStorage.removeItem('desiredRememberMeState');
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the reasoning behind clearing it? (I ask because I thought part of the goal was for it to stay how the user set it, even on subsequent logins, but that may have been in a separate YT issue.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So it will rely on the server side rememberMePreference cookie which I put in place earlier. The only issue is I think that will expire in 30 days possibly at the same time the rememberMe cookie expires so I could either remove the server side cookie and not remove this or I could set the expiration date later or indefinite. Any preference?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see any need for this to be done server-side (since I am not aware of us storing/using that preference anywhere server side), so I would lean towards just using the client-side local-storage option, with no expiration. I think that fits our goal better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants