-
Notifications
You must be signed in to change notification settings - Fork 11
Refactor/to scss title #271
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
✅ Deploy Preview for webdevpathstage ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
briangesteban
left a comment
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.
Looks good to me. Thanks @oluwatobiss for fixing the horizontal page scroll as well!
Just a question though, I'm curious of the teams decision of having a micro component like this. It seems that we could simplify it by just having the actual <h2> element directly on components that are using the <Title> component. And just style it globally or on the components' SCSS module. Is there a reason for the teams decision of having it?
Satoshi-Sh
left a comment
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.
Looks good to me. Thanks for fixing the styling bug
Good point. I see both approaches in the codebase. We might need a refactor for consistency at some point. |
Hmm. Great question. It could truly simplify it. I will implement this now for the team’s review. Thanks for sharing your thoughts, @briangesteban. |
This ensures the post content's overflowing h2 does not cause horizontal scroll
|
Refactoring the
|
briangesteban
left a comment
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.
Works just fine. Thanks for considering the refactor @oluwatobiss!

Have you updated the CHANGELOG.md file? If not, please do it.
Yes
What is this change?
blogTitleprop inblog/index.jsWere there any complications while making this change?
No
How to replicate the issue?
Steps to replicate the inappropriate horizontal scroll issue:
productmanagementtag.If necessary, please describe how to test the new feature or fix.
Compare the
<h2>headings of the “/blog/*” and “/about” pages on themainandrefactor/to-scss-snippets-titlebranches. The two should have the same style. Additionally, overflowing headings of the PR branch should auto-wrap to prevent inappropriate horizontal scrolling.When should this be merged?
After three approved reviews.