Olivero: submit button is too wide in the off canvas dialog box

This is tagged for an issue summary update. For this issue that looks suitable for a novice, adding tag. For more information see Write an issue summary for an existing issue → for guidance.

Aditi Saraf → made their first commit to this issue’s fork.

Patch for 11.x.

We, the mentoring team, are triaging issues for first-time contributors at DrupalCon Lille and I think this is a good issue for the contribution day.
We are reserving this issue so please don't work on this issue if you are not at DrupalCon Lille. You can continue the work when the event is over.

@kristofvercruyssen @simontrivali and me will be working on this issue at DrupalCon Lille with out mentors @anushrikumari and @shriaas

Patch seems to work perfectly!

Screenshots before patch:

Screenshots after the patch:

Issue summary updated as of comment #19

This patch seems to be working. Moving to RTBC.

+++ b/core/themes/olivero/css/components/button.pcss.css
@@ -106,6 +106,13 @@
+#drupal-off-canvas input[type="submit"] {
+  &.button {
+    float: left;
+    width: auto;
+  }
+}

Floats should only be added as a last resort - and that would likely only be in situations where floats are already in use nearby. Floats can lead to layout problems and accessibility bugs and CSS has added many new features to make floats unnecessary.

This should instead use a modern alternative such as flexbox for positioning.

Updating attributions

attribution updated.
trying to address #21

I'm setting the status of this issue to "Needs review" as the patch is updated.

Hi, I have verified this issue, and it looks fine to me.

Testing Steps
1. Go to your Drupal site.
2. Go to any page with an off canvas dialog with a submit button. ( For example in the module Layout Builder.)
3. Observe the size of the button

Testing Results
Patch applied cleanly and Olivero: submit button isn't looking wide in the off canvas dialog box

Attaching before and after screenshot.

Hence it can be move to RTBC
RTBC++

Thanks!

Fixes should be in MRs

Leaving issue summary tag as the User interface changes section is still empty.

The button should be disabled when making a post if any required field in the form is empty.

#26 is still needed

#21 also needed. The only change made since then was changing the order of CSS rules - nothing to address the feedback.

@akshaydalvi212 thanks for working on it but please read the comments @28 mentions that #21 still needed and #29 mentions #21 still needed

From what I can tell the patch was just turned to an MR.

I've updated MR!9477.

There are no reasons for "float: left".
Changed the width of the button to auto and restricted max-width to 100%.

Please review.

jimiobrien → changed the visibility of the branch 3213995-olivero-submit-button to hidden.

rodrigoaguilera → changed the visibility of the branch 3213995-olivero-submit-button to active.

@jimiobrien Was hiding the branch an accident?

A look at the failing tests is needed

It was correctly pointed out in #6 that this is not specific to Olivero, but the issue summary was still specifying that. The solution will likely be inside the off-canvas reset CSS in core/misc/dialog/off-canvas/css/

Just thinking.. In /web/core/misc/dialog/off-canvas/css/button.css the #drupal-off-canvas-wrapper .button is set to a width: 100%;.
If the dialog is on the right side of the (desktop) screen I guess it's designed to be as wide as the dialog. If the dialog is at the top or at mobile, it indeed looks a bit strange. I think it would look strange in all themes. Wouldn't it then be better to fix this in the dialog off-canvas button css file and maybe remove the width: 100% over there, or alternatively check if dialog is at left/right side and set the width to 100%?

Going to work on this issue on DrupalCon Barcalona 2024. Chris Darke was my mentor. Discussed this issue with Ben. Going to start a new branch and start from there.

I've created the Merge Request. Can someone review it? https://git.drupalcode.org/project/drupal/-/merge_requests/9648

bnjmnm → closed merge request !9477

I have been helping groendijk with this issue

The cspell error in DrupalCI appears unrelated to the MR. As far as the code changes go, this is RTBC but (as one might assume) should not be committed until whatever underlying issue causing the cspell failures is addressed. We can try re-running it later.

It may take a bit to address the gitlab thing so changing status as to not confuse any committers - but this is effectively RTBC as soon as the gitlab diff is worked out and cspell runs green

bnjmnm → changed the visibility of the branch 11.x to hidden.

I was pleased enough by the code that I forgot some housekeeping stuff: The issue summary should provide before/after screenshots with the after using the current MR.

The MR itself is RTBC IMO

Housekeeping: Added 4 screenshots.

TY for the screenshots - I added them to the issue summary and this is back to RTBC.

nod_ → closed merge request !9648

Committed and pushed 808ceae426b to 11.x and 628226171fc to 11.0.x and 687a650a490 to 10.4.x and fc0b1b87739 to 10.3.x. Thanks!

Automatically closed - issue fixed for 2 weeks with no activity.