Skip to content

Implement remember login mode choice#312

Merged
bennothommo merged 5 commits intorainlab:masterfrom
KGE:kge-implement-remember-feature
Jul 3, 2019
Merged

Implement remember login mode choice#312
bennothommo merged 5 commits intorainlab:masterfrom
KGE:kge-implement-remember-feature

Conversation

@KGE
Copy link
Copy Markdown
Contributor

@KGE KGE commented Feb 5, 2018

Improves #269

Inspired by backend version

@LukeTowers I think this is a nice starting point

@KGE KGE changed the title Implement remember feature Implement remember login mode choice Feb 5, 2018
@manuelbua
Copy link
Copy Markdown

manuelbua commented Mar 1, 2018

This would be a nice addition, any chance to have this merged upstream @LukeTowers?

@LukeTowers
Copy link
Copy Markdown
Contributor

@manuelbua have you tested these changes locally? Could you send screenshots of the login form with the checkbox and the backend settings for this? Please test all three modes (Yes, No, Ask) and verify that they work correctly.

@manuelbua
Copy link
Copy Markdown

@LukeTowers, you explicitly asked for a PR resubmission and were also looking forward at merging it: i genuinely asked about it, because i thought it was going to happen.

@LukeTowers
Copy link
Copy Markdown
Contributor

LukeTowers commented Mar 1, 2018

@manuelbua it is going to happen; however I don't have time to test this myself personally right this moment so I'm asking you to do it for me. Apologies if there was a miscommunication in what I meant to say.

@Eoler
Copy link
Copy Markdown
Contributor

Eoler commented May 3, 2018

I tried it and it works (and most sensible setting is "ask", of course).
It goes to production site tomorrow... thanks, @KGE !

@Eoler
Copy link
Copy Markdown
Contributor

Eoler commented May 21, 2019

This goodness still works with the latest plugin release, @bennothommo

@bennothommo
Copy link
Copy Markdown
Contributor

@Eoler Are you saying that this PR works with the latest version of the plugin?

@Eoler
Copy link
Copy Markdown
Contributor

Eoler commented May 22, 2019

Exactly, @bennothommo (ok, I didn't try 5 years expiry ;)
Its very handy with e-commerce sites, where security matters more...

@bennothommo
Copy link
Copy Markdown
Contributor

Thanks for letting us know, @Eoler. I'll give it a test and merge it once reviewed.

@bennothommo
Copy link
Copy Markdown
Contributor

@KGE I have tested this and it works very well - great job on this. I've merged in the master branch to resolve some conflicts, and also changed the "Yes" and "No" terms for the remember mode to "Always" and "Never", to make it more clear what the options mean in terms of setting a persistent session.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants