Olivero: Modal dialog buttons not aligned (for example, in the "Leave preview" dialog)

The Needs Review Queue Bot → tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

Fixed CCF, attached interdiff for same.
Also we're not using any variables in the file, so I have removed the import of variable file as well.

Verified the patch #105 and tested it on Drupal version 10.1.x with Olivero theme. The patch works fine and I have added the before and after screenshots for reference.

#101 updated issue summary so removing that tag.

+1 For me as this does solve the issue at hand. Not moving as still need review from olivero maintainers.

This looks good, and I think would probably be fine, but the fact that we're targeting all ui-dialogs in Olivero is reason enough to make sure this is bulletproof. As much as the design looks better with the buttons in alignment, it's more important that we don't

.ui-dialog {
  & .ui-button {

    &:last-child {
      margin-inline-end: 0;
    }
  }
}

This is targeting all buttons that are the last child in their wrapper, including the close button. This doesn't appear to change anything as that button doesn't have a margin, but since we're already digging into the buttonpane and buttonset wrappers later, we might as well limit the scope of this rule.

.ui-dialog {

  & .ui-dialog-buttonpane {
    padding-inline-start: 0.2em;
    padding-inline-end: 0.2em;

    & .ui-dialog-buttonset {
      display: flex;
      float: none;
    }
  }
}

Since we're setting the buttonset to display:flex;, I think we ought to consider a few things:

1. If we add flex-wrap: wrap; to the buttonset element, that will allow buttons to wrap to the second line if they need it, before wrapping extremely long text and overflowing the button's boundaries.

   
   

2. If we space the buttons in the buttonpane using the gap property, we don't have to worry about margins on any hypothetical 3rd buttons.

Hello @andy-blum,

Added flex-wrap: wrap in ui-dialog-buttonset for global implementation.

Please review.

I know it's sandwiched between the images, but still needs point #2 from my earlier review:

If we space the buttons in the buttonpane using the gap property, we don't have to worry about margins on any hypothetical 3rd buttons.

added patch and fixed CCF #109

Hello @andy-blum,

Added gap property for button layout to handle global implementation.

Please review.

This looks good to me. Thanks @pradipmodh13

The last submitted patch, 113: 3149863-113.patch, failed testing. View results →

Seems to be random failure.

Committed 1d97f05 and pushed to 10.1.x. Thanks!

It looks like manual testing was done in #144. I am removing the tag.

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