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

Switch from errors to alerts so oauth-related flashes will show #362

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

courte
Copy link
Member

@courte courte commented Mar 10, 2015

Errors weren't showing before, which kept users from seeing helpful information about having a public email address on GitHub and valuable feedback that their login attempt has occurred.

I can't screenshot this change because I don't have a GitHub account that replicates the login error, but testing flashes in the dashboard verified that flash[:error] does not show a result, while flash[:alert] shows up in red at the top of the page.

The other option was to add flash[:error] to the application layout, but it doesn't have the styling that alerts do, so this seemed like the better option.

@@ -76,7 +76,7 @@ def create
sign_in_and_redirect(:user, user)
end
else
flash[:error] = service_route.capitalize + ' cannot be used to sign up on CodeMontage as no valid email address has been provided. Please add a public email address or sign up manually. If you already have an account, you can sign in and add ' + service_route.capitalize + ' from your profile.'
flash[:alert] = service_route.capitalize + ' cannot be used to sign up on CodeMontage as no valid email address has been provided. Please add a public email address or sign up manually. If you already have an account, you can sign in and add ' + service_route.capitalize + ' from your profile.'

Choose a reason for hiding this comment

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

Line is too long. [309/80]
Unnecessary spacing detected.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

@courte
Copy link
Member Author

courte commented Mar 11, 2015

This fix is related to #288. The existing error messages have information about needing a public email address on GitHub, but they are not showing up to users because they're not flashed appropriately.

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