-
Notifications
You must be signed in to change notification settings - Fork 19
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
Add margin_top flag to feedback component #222
Conversation
cf02572
to
b875451
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple of tiny change suggestions
@@ -1,8 +1,10 @@ | |||
<% | |||
contact_govuk_path = "/contact/govuk" | |||
margin_top||= 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing space before the ||
@@ -1,8 +1,10 @@ | |||
<% | |||
contact_govuk_path = "/contact/govuk" | |||
margin_top||= 1 | |||
margin_top_class = "gem-c-feedback--top-margin" if margin_top == 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure you need the
== 1
Topic pages need the feedback component to be flush with the bottom of the taxon grid. We therefore need to be able to add a flag to remove the top margin in these cases
b875451
to
25ecf4d
Compare
@maxgds Added the missing space 👍 For the second comment, I think the |
I thought if it was set to false it would be the same as not existing in the evaluation, but happy to take your word for it. |
@maxgds Ah okay, I was following a pattern for another component where we pass |
Topic pages need the feedback component to be flush with the bottom of the taxon grid. We therefore need to be able to add a flag to remove the top margin in these cases
Example use on a topic page: