Umami uses themed unformatted lists to get a 'responsive grid layout', but since 10.0.0, we've had an actual responsive grid in core, so we should use that instead.
If we're lucky, this might just be reconfiguring the views, then locating and deleting any unnecessary CSS.
| Comment | File | Size | Author |
|---|---|---|---|
| #20 | Screenshot 2024-05-14 at 1.52.27 PM.png | 2.17 MB | smustgrave |
| #20 | Screenshot 2024-05-14 at 1.52.21 PM.png | 479.43 KB | smustgrave |
| #15 | 3425104-nr-bot.txt | 90 bytes | needs-review-queue-bot |
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes, plain diff MR !6970
Comments
Comment #2
catchFound various unformatted lists that are simulating a grid, as well as the legacy views grid style.
Converted all of these in the MR but have not touched the CSS.
The recipes collection looks broken now - the links are green, probably needs a class or two changing.
I'm sure we can remove some now-unnecessary CSS that was styling the unformatted lists but will leave that to the interested reader since CSS isn't my strength.
Comment #3
catchComment #5
finnsky commentedGonna check css here
Comment #6
finnsky commentedGreat initiative!
Please review
Comment #7
smustgrave commentedNice! Appears to have test failures though :(
Comment #8
finnsky commentedSomething wrong here with css_class schema. tried with both boolean and false.
Comment #9
catchviews config schema says it should be boolean - did you try installing and then re-exporting the view?
Comment #10
finnsky commentedseems that css class should be configured in admin ui and then reexported. probably it will help
Comment #11
finnsky commentedAll green!
Comment #12
catchI'm not qualified to review the actual CSS changes here, but the net reduction is great and shows just what an excellent feature responsive grid is!
Comment #13
catchRebased, pretty sure this will fail performance tests now for Umami since CSS coverage for added - but we just need to reduce the CSS byte assertions because there'll be less. Having trouble with chromedriver locally at the moment so letting gitlab tell us the new numbers.
Comment #14
catchBack to green, 1-2kb reduction in CSS, shows how useful the feature is both for reducing similar-ish code all over the place as well as reducing page weight on sites.
Comment #15
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #16
finnsky commentedComment #17
smustgrave commentedSeems to have failures in the openTelemetry tests which I believe (correct me if I'm wrong) isn't random.
Comment #18
catchIt's fine to set the ceiling on the OpenTelemetry tests .5k higher than whatever the eventual number is, that way other small CSS/JS changes won't cause this MR to fail.
Comment #19
finnsky commentedComment #20
smustgrave commentedFinally got around to this one
Did a fresh Umami install without the MR applied to verify column shifts on pages like /articles
Did a fresh Umami install with the MR already applied so the config imported
Visited /articles page to verify the layout is the same and responsive at various breakpoints
Then I did the same for the homepage as I saw that view was updated.
Also did not notice any issue.
Believe this is a good replacement.
Comment #26
nod_Since this only impacting umami I'm backporting back to 10.3.x
Committed and pushed 587350c208 to 11.x and f607b858d6 to 11.0.x and 57a0cd753d to 10.4.x and b6026c1215 to 10.3.x. Thanks!