Add bottom margin to success alert component #573
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
alex-ju
suggested changes
Oct 10, 2018
There was a problem hiding this comment.
Hi @1pretz1! Thanks for working on this.
I don't think we should provide this as an optional attributes. We should strive to have consistent spacing across components, hence impose it on every component. I know that some components - coming from the past (e.g. button) - use this option, but we should avoid introducing such attributes in the 'API'.
Happy to discuss it if that helps and sorry for not being around to explain these things from the beginning.
1pretz1
force-pushed
the
add-bottom-margin-option-to-alert
branch
from
f84f012
to
9d8d98d
Compare
1pretz1
changed the title
|
Hey @alex-ju, i've altered the pr to set a default for the bottom margin of 20px. |
1pretz1
added a commit
that referenced
this pull request
Oct 11, 2018
This fixes a bug where there was no bottom margin on the success-alert component (PR #573)
chao-xian
added a commit
to alphagov/content-data-admin
that referenced
this pull request
Oct 16, 2018
A change to the the govuk_publishing_components[1] removed mixins that our app use. To get around this, adding in govuk-frontend and then explicitly calling locally the missing mixins fixes some of the missing styles. [1] alphagov/govuk_publishing_components#573
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds a bottom margin by default to the success alert component.
Before
After
Trello:
https://trello.com/c/DYwko3da/313-no-margin-below-the-flash-banner-and-the-content-block-below