Skip to content
This repository has been archived by the owner on Sep 4, 2018. It is now read-only.

Fix notification when signing up (or really any flash) #649

Merged
merged 4 commits into from Feb 25, 2017

Conversation

technicalpickles
Copy link
Contributor

This is one possible fix for #646

Another problem I observed is when there are multiple alerts on a page (by having, say, both flash[:notice] and flash[:error]). The .alert style uses a z-axis, so the multiple alerts would be positioned on top of each other, and all but the first would be hidden.

This PR solves both by putting the alerts into it's own section (but I guess it could have been a container too). This allows the section to be positioned (with z axis, offset from the top), and multiple alerts can go inside

top: $navbar-height;
position: fixed;
left: 50%;
}

Choose a reason for hiding this comment

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

Rule declaration should be followed by an empty line

@@ -92,17 +94,21 @@ i.fa.medium {
padding-left: 3%;
}


#alert-section {
z-index: 997;

Choose a reason for hiding this comment

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

Properties should be ordered left, position, top, z-index

@@ -92,17 +94,21 @@ i.fa.medium {
padding-left: 3%;
}


#alert-section {

Choose a reason for hiding this comment

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

Avoid using id selectors

@sethbergman
Copy link
Member

Very nice fix! 👍

@technicalpickles
Copy link
Contributor Author

I know there is some work for changing signup to have a landing page (#635), but I think this will still be useful for anytime flash is used.

@hollomancer hollomancer merged commit bd982c6 into OperationCode:master Feb 25, 2017
@technicalpickles technicalpickles deleted the fix-hidden-alert branch February 26, 2017 23:42
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants