Use CSS grid for gallery styles #1246
Conversation
|
Unfortunately, for now, |
|
I wish |
|
Thank you @gooklani for the pull request. From the code styling perspective, we use tabs and not spaces so that would need to be changed. As we currently have CSS and Sass files, both would need to be changed. Could you mention what browser support is needed for this feature so that we can discuss it? A screenshot of the before and after would be helpful to see any changes. Thank you! |
|
For browser support, could the @supports (display: grid) {} rule help? Or use css hierarchy by implementing css grid before the older css rules to allow overrides. |
Internet Explorer 11 will never be updated to the new specification, and this is why grid layouts are not supported in IE. Here's the complete picture: @bahiirwa good idea, the @supports CSS at-rule enables us to specify declarations which depend on a browser's support for one or more specific CSS features. @grappler What do you think? Would you like me to implement this so far? |
|
I think it is good to use modern CSS features. As IE still has 3% market share we need to make sure there is a backwards compatible solution. Also there are a few mobile browser that do not support it also. It would be good to get more feedback from Frontend Developers. @mor10 has been pushing CSS grid. It would be interesting to hear his thoughts. |
|
Grid is the right layout module here. If a fallback is necessary, the best option is to provide non-grid rules as the default and wrap the grid rules an an @supports rule. However, there's a philosophical question that needs to be addressed here which has implications beyond the gallery: Should grid be used as the primary layout module for _s as a whole? If it's used in the gallery, it should be used everywhere which means _s needs to be refactored substantially. This in turn begs the question of why _s should not use grid as it's primary layout module. The immediate answer will be backwards compatibility, but is this a good reason? Is it _s' job to delay implementation of modern best practices for the sake of backwards compatibility? My personal opinion is no. _s should provide a modern baseline for development and the individual theme developer who uses it has the responsibility of providing whatever backwards compatibility is required for their project. |
|
💯 for refactoring _s layout as a whole. Floats have needed to go for a while, IMO. #682 And grid is surely the way to go for that as it makes short work out of complex layouts. But for something as simple as this basic gallery where the items are equal in size, what's the benefit of grid over the current inline-block implementation or even flexbox? I've not used grid a ton but it appears both the behavior and the amount of code would be equal with this PR. Is that correct? |
|
@mor10 that personal opinion makes sense. It defines a starter theme. Not completing the job for developer. "implementation of modern best practices" should be key and where possible we should do backward compatibility. |
|
Great idea to use CSS grid for the gallery and maybe for the whole _s theme.
|
|
CSS Grid solves the layout structure problem by allowing designers to control both columns and rows simultaneously and in addition to that, you would be able to use Grid Gap to manage the spacing between the elements instead of tricky implementation of margin or padding. @m-e-h I'm not sure why you are strictly taking sides against this grid implementation but to me being on the cutting edge of development is something we should want to do from anywhere or anyplace we could. I assume this could be a start of something new and we can extend it to cover all aspects of the theme layout eventually. |
|
On hind sight, there were issues to do with implementing framework like bootstrap etc in _s. Common consensus was leave that to developer. Doesn't using grid in the whole of _s going to brig issues with a developer choosing to use a CSS framework? |
|
@m-e-h like @gooklani says, using grid for the gallery opens the door for more advanced layouts, and more responsive ones too. For example, it would not be hard to make a gallery layout where some of the images are twice the size of others, and even make this dependent on how many images are in the gallery. It also makes it easier to style the gallery as a whole (because it wraps and clears properly) and gallery items individually. |
|
I have a fully refactored version of _s set up for grid in a private repo already. I can clean it up and submit it as a pull requests next week so you can all take a look. A lot is changed including nesting, clearings, and other features. |
|
@bahiirwa CSS frameworks like Bootstrap and Foundation were introduced to provide a solution to the lack of proper grid layout tools on web platform. Now that CSS Grid is live, the framework approach of complex HTML-based grids is no longer recommended, and actively works against the way the web works. Some frameworks will adopt grid as their layout module. Others will stick with old methods and will hold bot themselves and the web back as a result. In my opinion it would be counterproductive to avoid grid to preserve support for outdated methods. |
|
I'm all in for using Grid everywhere even if it's pretty basic stuff in |
Updated gallery item styles to generate a block-level grid
* 'master' of https://github.com/Automattic/_s: (92 commits) Remove PHPCS Ignore Around $content-width (Automattic#1450) Use _S_VERSION constant when enqueuing customizer.js (Automattic#1445) Update composer and node dependencies (Automattic#1444) Add missing s in languages directory name (Automattic#1442) Exclude alignment classes from RTL conversion (Automattic#1435) Add new required header fields for style.css (Automattic#1432) Remove redundant font-size declarations (Automattic#1434) Refactor navigation.js to Javascript ES6 (Automattic#1423) Delete the custom bundle script in favor of an npm package (Automattic#1433) Fix small typo (Automattic#1428) Re-organize SASS structure (Automattic#1425) Add a SASS watcher command (Automattic#1427) Remove the skip link focus fix (Automattic#1424) Replace className.indexOf with classList.contains (Automattic#1422) Use CSS grid for gallery styles (Automattic#1246) Update README.md documentation (Automattic#1419) Add a bundle script to generate a zip file for theme distribution (Automattic#1417) Update npm dependencies (Automattic#1418) Float implies the use of the block layout (Automattic#960) Use the system font stack from WP admin (Automattic#1416) ...
Changes proposed in this Pull Request:
Updated gallery item styles to generate a block-level grid
Related issue(s):