Skip to content
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

Update hero section styles according to BEM methodology #1107

Merged
merged 2 commits into from
Oct 30, 2024

Conversation

nafisreza
Copy link
Contributor

fixes #1102

Changes:

  • All the hero section classes have been updated according to BEM.
  • Respective HTML classes have also been updated.
  • Visual test has been updated with new class name.

Copy link
Contributor

github-actions bot commented Oct 26, 2024

🚀 Regression report for commit 0ceadab is at https://web-php-regression-report-pr-1107.preview.thephp.foundation

Copy link
Contributor

github-actions bot commented Oct 26, 2024

🚀 Preview for commit 0ceadab can be found at https://web-php-pr-1107.preview.thephp.foundation

styles/home.css Outdated Show resolved Hide resolved
@lhsazevedo
Copy link
Contributor

Everything looks great to me, just a minor suggestion (nit) in the comment I added. Once that's addressed, I think it should be good to go!

@nafisreza
Copy link
Contributor Author

Thanks @lhsazevedo. I accidentally overlooked it.
Fixed in the last commit

@lhsazevedo
Copy link
Contributor

LGTM


Note for the person merging: Please either add the hacktoberfest-accepted label to this PR, or follow the suggestion in #1106 to add the hacktoberfest label to the repository.

Copy link
Member

@sy-records sy-records left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@sy-records sy-records merged commit cb385e3 into php:master Oct 30, 2024
5 checks passed
@sy-records sy-records changed the title feat: update hero section styles according to BEM methodology Update hero section styles according to BEM methodology Oct 30, 2024
@nafisreza
Copy link
Contributor Author

Thanks a ton, @sy-records, and @lhsazevedo.
Do you think the rest of the website could also be updated according to BEM? Maybe one by one. If yes, I would love to contribute.

lhsazevedo added a commit to lhsazevedo/web-php that referenced this pull request Oct 31, 2024
Hero classes changed after php#1107. Should fix visual tests.
@lhsazevedo
Copy link
Contributor

lhsazevedo commented Oct 31, 2024

Thanks you!

The rest of the website could indeed be refactored to follow BEM, which would help a lot with readability and maintenance. However, there's quite a bit of technical debt in the styles that goes beyond just the naming convention. We're dealing with some older techniques, like float-based layouts with clearfixes, which could definitely use a modern overhaul.

Switching to Flexbox (and potentially Grid where it makes sense) would significantly improve the layout's flexibility and remove a lot of that legacy CSS. Converting to BEM alongside these structural updates would make everything cleaner and more modular.

Appreciate the offer to help! It might be best to tackle it one section or block at a time, updating both the structure and the naming conventions incrementally.

Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Convert homepage hero section styles to use BEM methodology
3 participants