Fix style of banner #3920
Fix style of banner #3920
Conversation
There was a problem hiding this comment.
@edumoreira1506 Thank you very much for contributing to CONSUL 😄.
Sorry for the late review 🙏. We were preparing version 1.1 when this pull request was opened, and after that it's taken us a while to catch up.
I've left a few comments. Let me know what you think! Also, don't be afraid to disagree with what I say 😉.
| .banner { | ||
| margin-bottom: 2rem; | ||
|
|
||
| &__container { |
There was a problem hiding this comment.
Instead of adding an extra container, do we get the same results if we apply these styles to the a tag inside the .banner? If so, I'd rather go for it; we already have too many div tags in our views 😅.
| @@ -9,7 +9,7 @@ | |||
| <% end %> | |||
|
|
|||
| <% if current_budget.present? %> | |||
| <div id="budget_heading" class="expanded budget no-margin-top"> | |||
| <div id="budget_heading" class="expanded budget"> | |||
There was a problem hiding this comment.
Note removing this class will add an extra margin when there's no banner.
Also, since now we're adding a bottom margin to the banner, IMHO this change isn't necessary 🤔. What do you think?
| @@ -0,0 +1,11 @@ | |||
| .banner { | |||
| margin-bottom: 2rem; | |||
There was a problem hiding this comment.
This one is tricky because the margin is needed in some sections but not in other sections 🤔. I guess it's better to have too much margin in places like the proposals section rather than too little in places like the help page, so I'm fine with this change 👍.
|
Hi, @edumoreira1506 😄. Are you still working on this pull request? 🤔 |
|
Due to inactivity, closing in favor of #4080. |
References
Objectives
This PR fixes banner style in all pages. You can see all broken styles in #3639
Visual Changes
Before of the changes, in home page, the banner was outside of site container and now:
Before of the changes, in help page, the banner wasn't bottom margin, and this caused an impression that banner was together with default banner of help page, and now:
Before of the changes, in budgets page, banner was broken, and now:
