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

Working logout function #754

Merged
merged 2 commits into from
Jun 30, 2015
Merged

Working logout function #754

merged 2 commits into from
Jun 30, 2015

Conversation

jrjohnson
Copy link
Member

When a user logs out they get redirected to the login page with a flash message confirming the logout.

Fixes #753

When a user logs out they get redirected to the login page with a flash
message confirming the logout.
@saschaben
Copy link
Member

can we move the login form down on the page about 60px, so the "login" label doesn't conflict with the flash message? With that and a code review this should be fine to push.

I think this is fine for the beta; we should probably review (with JenZ) to determine a better long term display model here; the flash message is a bit short for a message like this, probably want something just a little more persistent in this particular situation.

@jrjohnson
Copy link
Member Author

The form is styled without anything special right now. I would prefer to merge and leave it that way and then take some time to completely restyle the experience later. (for example error messages are a bit off).

@saschaben
Copy link
Member

That's fine, I'd still like to move it down the page 60px, as the overlay looks more like an accident right now, and less like a WIP. But if you are adamant, I'm OK. We'll just put it on the list for 0.5.0.

This puts it underneath the logout confirmation flash message.
saschaben added a commit that referenced this pull request Jun 30, 2015
@saschaben saschaben merged commit 79b450c into ilios:master Jun 30, 2015
@jrjohnson jrjohnson deleted the 753-logout branch June 30, 2015 19:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants