-
Notifications
You must be signed in to change notification settings - Fork 1.7k
fix:improve footer responsiveness on smaller screens #3626
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
Conversation
|
Hi @martin-g , this PR improves footer responsiveness on small screens. Please take a look. |
doc/assets/scss/_styles_project.scss
Outdated
| margin-right: auto; | ||
| display: block; | ||
| } | ||
| } No newline at end of file |
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.
| } | |
| } | |
doc/assets/scss/_styles_project.scss
Outdated
| font-family: "PT Mono", monospace; | ||
| } | ||
|
|
||
| @media (max-width: 996px) { |
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.
Why 996 ?
https://getbootstrap.com/docs/5.0/layout/breakpoints/ uses 992
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.
Updated the breakpoint to 992px to match Bootstrap’s lg breakpoint.
doc/assets/scss/_styles_project.scss
Outdated
| display: flex; | ||
| flex-direction: column; | ||
| align-items: center; | ||
| text-align: center; |
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.
| text-align: center; |
This one seems redundant. The one at line 49 will override it anyway.
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.
Agreed, removed the redundant text-align rule.
doc/assets/scss/_styles_project.scss
Outdated
| text-align: center; | ||
| } | ||
|
|
||
| footer [class^="col-"] { |
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.
This will match only if "col-..." is the first item in class="...", e.g. class="col-3", but it won't match for class="another col-3"
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.
Thanks for pointing this out. Updated the selector to avoid relying on class name prefixes.
doc/assets/scss/_styles_project.scss
Outdated
| } | ||
|
|
||
| footer [class^="col-"] { | ||
| width: 100% !important; |
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.
!important is usually not recommended. Did you try without it ? E.g. with more concrete selector.
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.
Removed !important and adjusted the selector accordingly.
doc/assets/scss/_styles_project.scss
Outdated
| margin: 0.4rem; | ||
| } | ||
|
|
||
| footer img { |
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.
Is this needed ?
<img> is an inline element, so the text-align: center from line 49 should be enough.
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.
removed the unnecessary img centering rule.
|
Thanks for the detailed review! I’ve noted the points and will address them shortly with an updated commit. |
0d0baaf to
ecaba14
Compare
|
Thanks for the review! @martin-g . I’ve pushed an update addressing all the changes you suggested and the final footer layout is same as with the previous css code. Please let me know if this looks good now. |
|
Thank you, @Prakash1185 ! |
Problem
What changed
Before
After