Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Dialog header content wraps below title, or is pushed outside dialog #4309

Closed
rolfsmeds opened this issue Aug 5, 2022 · 4 comments · Fixed by #4310
Closed

Dialog header content wraps below title, or is pushed outside dialog #4309

rolfsmeds opened this issue Aug 5, 2022 · 4 comments · Fixed by #4310
Assignees
Labels

Comments

@rolfsmeds
Copy link
Contributor

rolfsmeds commented Aug 5, 2022

Description

If the dialog's title is too long to fit next to the header-content part (containing typically a close button), the header-content part wraps below the title, which looks silly, and is hardly what the developer intended:

image

Even worse, if the title contains a sufficiently long substring, the header-content part can be pushed entirely outside the dialog:
image

Expected outcome

Header-content stays next to the title no matter what.

  • The wrapping below the title is easily achieved by setting flex-wrap: nowrap on the header part.
  • The pushing outside of dialog could be fixed e.g. by setting overflow:hidden and, ideally, text-overflow:ellipsis on the title part.

Minimal reproducible example

Wrapping:

Dialog dialog = new Dialog();
dialog.setHeaderTitle("A sufficiently long dialog title that pushes the header-content part below");

Button closeButton = new Button(new Icon("lumo", "cross"), (e) -> dialog.close());
closeButton.addThemeVariants(ButtonVariant.LUMO_TERTIARY);
dialog.getHeader().add(closeButton);

Pushing outside:
Replace title with "ASufficientlyLongDialogTitleThatPushesTheHeader-contentPartOutside"

Steps to reproduce

Make the viewport of the above dialog sufficiently narrow for the title to wrap / push outside.

Environment

Vaadin version(s): 23.1

Browsers

Issue is not browser related

@rolfsmeds rolfsmeds changed the title Dialog header content wraps below title Dialog header content wraps below title, or is pushed outside dialog Aug 5, 2022
@web-padawan web-padawan self-assigned this Aug 5, 2022
@jcgueriaud1
Copy link
Contributor

Normally this css should work as a workaround:

vaadin-dialog-overlay::part(header) {
    flex-wrap: nowrap;
}

vaadin-dialog-overlay::part(title) {
    overflow: hidden;
}

vaadin-dialog-overlay > span[slot="title"] {
    white-space: nowrap;
    overflow: hidden;
    text-overflow: ellipsis;
    display: block;
}

@web-padawan
Copy link
Member

The problematic CSS was added by #3654. It makes sense for footer where users often tend to place several buttons and expect them to wrap in multiple lines on small screens. But we probably should not apply it to header part.

@jouni
Copy link
Member

jouni commented Aug 5, 2022

I did add that CSS intentionally (allow the header content to wrap), thinking it would be the most flexible and least brittle.

But perhaps it should be changed, if we assume the most common use for the header content area is just a close button.

@rolfsmeds
Copy link
Contributor Author

rolfsmeds commented Aug 8, 2022

I do see the rationale in that, but I feel that it's optimizing for the wrong thing.

The way I see the header-content slot was mainly for the following use cases:

  1. Close buttons and potentially other buttons commonly placed in the top-right of a dialog
  2. Providing completely custom header content instead of the built-in title (e.g. if you want to have an icon, a title with a different-than-default heading level, and some buttons)

In case 1, you probably don't want the close button (or the other buttons) to wrap below title.
In case 2, there is no title – header-conent takes up the entire header – so it doesn't really matter how the header wrapper lays things out.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants