Closed Jacfem closed 6 years ago
Let’s get this at the bottom of the actual press page instead. Above the footer.
@kylemh Cool, I think it would look more logical there as well. Before I push the change, wanted to point out that this is how it would look on mobile and desktop, respectively. Would you be interested in me removing the min-height: 250px
from the Branding
section? It would then look like the second set of pictures. No worries if not, I just thought it looked like a bit too much empty space.
@Jacfem
I wasn't thinking ahead on this issue, but now that the PR is open, I've chatted with Conrad and we have a clear picture of what we'd like for now.
Let's try to implement it like this:
RE: Removing minimum height.
I don't think it's worth the effort, but you're welcome to make the change to height: auto with no min-height. Please do not change the Section component and do not use !important
@kylemh No problem, thanks for all the clarification! I believe I've addressed everything - regarding the thought around removing the hard-coded min-height, I decided it seems out of scope for this and I made no changes there.
Let me know if anything else should be addressed, thank you!
Updated pictures:
Looks great toe me!
LGTM!
Description of changes
-Adds the CivicX image below the footer on the Press page, per the request in this Issue. -Also removes some conflicting footer height rules, and addressed some linting warnings.
Issue Resolved
Fixes #1014