TheOdinProject / css-exercises

MIT License
1.69k stars 71.51k forks source link

03-flex-header-2: remove .left,.right selectors in solution.css & respective divs in .html #544

Closed rishikpogu closed 6 months ago

rishikpogu commented 6 months ago

Because

The previous solution included some changes in html (creating new divs & classes), which are now excluded in this solution and this helps in using flexbox a better way.

This PR

Issue

Closes #XXXXX

Additional Information

Pull Request Requirements

MaoShizhong commented 6 months ago

Thanks for opening this PR @rishikpogu.

I'm personally not convinced the current solution needs to be changed. I think it's sufficiently intuitive - you have items you want to group on the left side, and items to be grouped on the right, so you make a container for each and space-between them to either side. This is a fairly common way to do things, because often you will purposely want some elements to be grouped together, then have the groups moved. As opposed to just having all elements as individual items and just making the "left" and "right" items move to each side via the space in the middle (which means they might be on specific sides, but they're not actually grouped with things we might want them to be grouped with). And putting margin-left: auto on the button element selector wouldn't be sensible since that would affect every button (only one in this case, but it wouldn't be sensible to do that in an example solution specifically, in my opinion).

What you have does work though, and does meet the requirements of the task, so well done on this! Only saying for the PR specifically that the current solution I think suffices for the reasons shared above, so I'll close this as these changes won't be merged for it.