-
Notifications
You must be signed in to change notification settings - Fork 1
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
fix: updated homepage spacing #1414
Conversation
✅ Deploy Preview for detroit-public-dev ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for detroit-partners-dev ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for detroit-storybook-dev ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
sites/public/styles/footers.scss
Outdated
@@ -4,8 +4,9 @@ footer.site-footer > .footer-row:first-of-type { | |||
} | |||
|
|||
.footer-sections { | |||
padding: 2rem 0; | |||
max-width: 1024px; | |||
@apply py-8; |
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.
We're moving off of tailwind, do you mind adding these as vanilla css classes since they're new?
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.
@emilyjablonski Just updated!
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.
@emilyjablonski We should talk about this. I don't know if vanilla CSS is the solution here. I think what we want is to create tokens using CSS variables so we can reuse this spacers across the site.
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.
I believe the tokens are available already…just need to swap out any remaining @apply
or hardcoded values (aka var(--bloom-s2)
rather than 0.5rem
)
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.
Visually LGTM, just one style comment
Pull Request Template
Issue Overview
This PR addresses bloom-housing#2927
Description
This PR updates the spacing on the coming soon section, the regions section, and the footer.
How Can This Be Tested/Reviewed?
This can be tested by using the Public preview and verifying that the changes reflect those seen in the issue.
Checklist:
yarn generate:client
and/or created a migration if I made backend changes that require themReviewer Notes:
Steps to review a PR:
On Merge:
If you have one commit and message, squash. If you need each message to be applied, rebase and merge.