Re-add base styling for footer actions to support simple customisations #11751

lb- · 2024-03-11T21:32:15Z

Overview

Fixes Footer action menu with extra_footer_actions needs better styling #11629
Relates to clean up done as part of Replace legacy dropdown component with new Tippy dropdown-button. Fix #7366 #11456 which removed a lot of styling for footer actions which appeared to be unused.
Re-add some base styling to support the ability for basic customisations where buttons are added within the extra_footer_actions block.

I'd recommend we also include this in 6.0.2 as it will help packages update to support Wagtail 6.0* easier.

Details

The PR referenced above did a lot of incredible work to clean up the complex styles in the footer, but it appears that some of that made basic customisations much easier.

This PR proposes we add some bare minimum styling back to support the common use case of showing a few buttons in the extra_footer_actions block.

Testing

Follow the steps in #11629 (comment)

Before

After

squash-labs · 2024-03-11T21:32:21Z

Manage this branch in Squash

Test this branch here: https://lb-fix11629-better-base-stylin-yj2pe.squash.io

zerolab · 2024-03-12T14:12:00Z

@lb- I wonder if the extra actions should be inline, to the right of the primary actions as we had up to 6.0?

lb- · 2024-03-12T21:04:01Z

@zerolab the new max width makes this very tight except for maybe one added button.

zerolab · 2024-03-12T21:16:57Z

Ah, that's unfortunate.
I guess the best thing we can do is advise people to register it as a page footer action. It is still worth for 1-2 extra buttons. Thank you

lb- · 2024-03-12T22:00:49Z

OK so should we close this PR or still attempt to account for a 'basic' usage here?

@zerolab

zerolab · 2024-03-13T12:53:52Z

@lb- this PR is an improvement over what we have now, so we should keep it. Sorry if I was unclear

laymonage · 2024-03-13T14:20:12Z

Regarding the max width, I haven't checked if this is possible, but we might be able to set the max width on just the dropdown, while using something like fit-content for the container. That might allow us to keep the max width in most cases, but still allowing additional items to appear with enough width side-by-side.

lb- · 2024-03-13T20:52:36Z

OK this is getting a bit more complex but this should support any arbitrary sibling elements to the dropdown while conserving the current intent of having a wider dropdown (to account for different label lengths).

I have also now accounted for small (mobile sized) devices supporting multiple buttons side by side and having the correct heights.

This may not work for really long translations and lots more buttons but should be ok.

- Fixes wagtail#11629 - Relates to clean up done as part of wagtail#11629 which removed a lot of styling for footer actions which appeared to be unused. - Re-add some base styling to support the ability for basic customisations where buttons are added within the `extra_footer_actions` block.

laymonage

Thanks @lb-!

This is a solid base for reinstating the styles, but I noticed a few issues. I have made the fixes locally, I'll push them to your branch and double check. If I don't find any more issues, I think I'll merge this so we can unblock external packages like wagtail-content-import.

laymonage · 2024-04-02T08:08:13Z

client/scss/components/_footer.scss

+    gap: theme('spacing.2');
+    grid-auto-flow: column;
+
+    > * {


From what I've seen we're only doing this to reset the default .button styles, so I think it's better to apply this more conservatively, e.g.

Suggested change

> * {

> .button {

laymonage · 2024-04-02T08:12:39Z

client/scss/components/_footer.scss

+    // Support basic layout support for items added via `extra_footer_actions`
+
+    align-items: stretch;
+    display: grid;


As I understand, this allows us to remove the w-grid from these:

wagtail/wagtail/snippets/templates/wagtailsnippets/snippets/create.html

Line 15 in 0599a56

<nav class="actions actions--primary footer__container w-grid" aria-label="{% trans 'Actions' %}">

wagtail/wagtail/snippets/templates/wagtailsnippets/snippets/edit.html

Line 16 in 0599a56

<nav class="actions actions--primary footer__container w-grid" aria-label="{% trans 'Actions' %}">

which were added after #11456 (comment).

laymonage · 2024-04-02T08:47:00Z

client/scss/components/_footer.scss

+
+    > * {
+      // Reset the margin on any .button sibling elements
+      height: auto;


Unfortunately this causes the button to be very small when there's no dropdown (e.g. when the page is locked)

and the button to be very large when it has an icon (due to missing height set for the icon):

I think we have to define our own height here instead of relying on the dropdown's button height, which in turn relies on the default .button height of 3em or the $text-input-height:

wagtail/client/scss/components/_dropdown-button.scss

Line 66 in 0599a56

min-height: $text-input-height;

Unfortunately, 42px or 2.625rem (the current height, which is also $text-input-height) is not defined in our spacing tokens. We can either add it as spacing[10.5] and use it, or use $text-input-height instead.

Since we already use $text-input-height in _dropdown-button.scss, I think we can use it here too. It would be nice to add it as spacing[10.5] in the future though, as it seems we use this value in quite a few places.

laymonage · 2024-04-02T09:02:51Z

client/scss/components/_footer.scss

@@ -103,6 +125,7 @@

  .icon {
    width: theme('spacing.4');
+    height: theme('spacing.4');


Without this, we had this issue (even before this PR) where the svg's height goes beyond the button's, but this wasn't noticeable until we added height: auto to the button in the earlier iteration of this PR.

laymonage · 2024-04-02T09:05:53Z

wagtail/snippets/templates/wagtailsnippets/snippets/create.html

-        <nav class="actions actions--primary footer__container w-grid" aria-label="{% trans 'Actions' %}">
+        <nav class="actions actions--primary footer__container" aria-label="{% trans 'Actions' %}">


Can confirm that this works OK even when there's only a single action

laymonage · 2024-04-02T09:08:50Z

client/scss/components/_footer.scss

+      // - set height responsively
+      // - add a margin when there is a .button sibling
+      // Unset these styles in favor of a fixed height and the grid gap
+      height: $text-input-height;


Tested a few different scenarios and this seems OK

what about cases where one wraps that .button in a form? wagtail-localize used to do that for the additional buttons (a la "import from local file")

Hmm, OK, good point. I wanted to remove the > but was afraid it would be risky.

The main dropdown action button from Wagtail is also wrapped inside another element, and without removing the > this technically still uses the default styles from .button (see bottom right), but the _dropdown-button.scss has its own min-height styles to handle the case for smaller screens so that it uses this same height instead of the smaller one:

I think it should be safe to remove the >.

Tried wrapping it in a <form> but the browser won't render it, probably because the footer actions are inside the page editor's form, and nested forms aren't valid HTML5.

Here's with a <div> instead (and > .button changed to .button)

Thanks for checking!

localize is a bit special in that it replaces the edit form with its React bits (one more reason to rewrite with good old panels).

Thanks everyone. A great solution in the end.

Team work! Thank you @lb- and @laymonage ❤️

as per wagtail/wagtail#11751

laymonage · 2024-04-02T09:58:03Z

Thanks @lb- @zerolab

main: f941b45
stable/6.0.x: de046a7

Will see about releasing 6.0.2 sometime this week.

lb- · 2024-04-02T21:56:20Z

Thanks @zerolab and @laymonage

as per wagtail/wagtail#11751

lb- added status:Needs Review component:Page labels Mar 11, 2024

lb- mentioned this pull request Mar 11, 2024

Footer action menu with extra_footer_actions needs better styling #11629

Closed

zerolab mentioned this pull request Mar 12, 2024

Wagtail 6.0 and Django 5.0 support torchbox/wagtail-content-import#63

Closed

lb- force-pushed the fix/11629-better-base-styling-for-footer-actions branch 2 times, most recently from 5ee324d to 3f7b9f0 Compare March 12, 2024 21:11

lb- force-pushed the fix/11629-better-base-styling-for-footer-actions branch from 3f7b9f0 to 138f441 Compare March 13, 2024 21:07

lb- requested a review from zerolab March 13, 2024 21:07

lb- force-pushed the fix/11629-better-base-styling-for-footer-actions branch from 138f441 to 09b30da Compare March 21, 2024 20:35

gasman added a commit to gasman/wagtail-content-import that referenced this pull request Mar 21, 2024

Hack in CSS fix from wagtail/wagtail#11751

c5910f1

gasman mentioned this pull request Mar 21, 2024

Complete Wagtail 6.0 / Django 5.0 support torchbox/wagtail-content-import#64

Merged

lb- force-pushed the fix/11629-better-base-styling-for-footer-actions branch from 09b30da to 8639000 Compare March 23, 2024 05:44

laymonage self-requested a review April 2, 2024 05:27

laymonage self-assigned this Apr 2, 2024

laymonage reviewed Apr 2, 2024

View reviewed changes

laymonage force-pushed the fix/11629-better-base-styling-for-footer-actions branch from 8639000 to ee4b699 Compare April 2, 2024 09:00

laymonage reviewed Apr 2, 2024

View reviewed changes

Apply suggestions from code review

c8686cc

laymonage added a commit to lb-/wagtail that referenced this pull request Apr 2, 2024

Release note for wagtail#11751

2d7429d

laymonage force-pushed the fix/11629-better-base-styling-for-footer-actions branch from ee4b699 to 2d7429d Compare April 2, 2024 09:40

zerolab added a commit to wagtail/wagtail-localize that referenced this pull request Apr 2, 2024

Tidy up action bar styles

16d3711

as per wagtail/wagtail#11751

Release note for wagtail#11751

f941b45

zerolab approved these changes Apr 2, 2024

View reviewed changes

laymonage force-pushed the fix/11629-better-base-styling-for-footer-actions branch from 2d7429d to f941b45 Compare April 2, 2024 09:54

laymonage merged commit f941b45 into wagtail:main Apr 2, 2024
11 of 14 checks passed

laymonage added a commit that referenced this pull request Apr 2, 2024

Release note for #11751

de046a7

laymonage removed the status:Needs Review label Apr 2, 2024

zerolab added this to the 6.0.2 milestone Apr 2, 2024

lb- deleted the fix/11629-better-base-styling-for-footer-actions branch April 2, 2024 21:54

zerolab added a commit to wagtail/wagtail-localize that referenced this pull request Apr 3, 2024

Tidy up action bar styles

9f23308

as per wagtail/wagtail#11751

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Re-add base styling for footer actions to support simple customisations #11751

Re-add base styling for footer actions to support simple customisations #11751

lb- commented Mar 11, 2024 •

edited by laymonage

squash-labs bot commented Mar 11, 2024

zerolab commented Mar 12, 2024

lb- commented Mar 12, 2024

zerolab commented Mar 12, 2024

lb- commented Mar 12, 2024

zerolab commented Mar 13, 2024

laymonage commented Mar 13, 2024

lb- commented Mar 13, 2024

laymonage left a comment

laymonage Apr 2, 2024

laymonage Apr 2, 2024

laymonage Apr 2, 2024

laymonage Apr 2, 2024

laymonage Apr 2, 2024

laymonage Apr 2, 2024

zerolab Apr 2, 2024

laymonage Apr 2, 2024

laymonage Apr 2, 2024

zerolab Apr 2, 2024

lb- Apr 2, 2024

zerolab Apr 3, 2024

laymonage commented Apr 2, 2024

lb- commented Apr 2, 2024

		<nav class="actions actions--primary footer__container w-grid" aria-label="{% trans 'Actions' %}">
		<nav class="actions actions--primary footer__container" aria-label="{% trans 'Actions' %}">

Re-add base styling for footer actions to support simple customisations #11751

Re-add base styling for footer actions to support simple customisations #11751

Conversation

lb- commented Mar 11, 2024 • edited by laymonage

Overview

Details

Testing

Before

After

squash-labs bot commented Mar 11, 2024

Manage this branch in Squash

zerolab commented Mar 12, 2024

lb- commented Mar 12, 2024

zerolab commented Mar 12, 2024

lb- commented Mar 12, 2024

zerolab commented Mar 13, 2024

laymonage commented Mar 13, 2024

lb- commented Mar 13, 2024

laymonage left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

laymonage commented Apr 2, 2024

lb- commented Apr 2, 2024

lb- commented Mar 11, 2024 •

edited by laymonage