vaadin / web-components

A set of high-quality standards based web components for enterprise web applications. Part of Vaadin 20+
https://vaadin.com/docs/latest/components
451 stars 83 forks source link

vaadin-radio-button is not center aligned #3342

Open TatuLund opened 2 years ago

TatuLund commented 2 years ago

vaadin-radio-button is currently baseline aligned

In many scenarios this is visible so that the circle is few pixels higher, center alignment could be visually more balanced (opionated)

Workaround

frontend/styles/radio-button-style.css

.vaadin-radio-button-container {
 align-items: center !important;
}

 Import in Java  @CssImport(value = "./styles/radio-button-style.css", themeFor="vaadin-radio-button")

jouni commented 2 years ago

Center alignment won't work with labels that wrap on multiple lines. That’s the main reason to use baseline at the moment.

Side note: the workaround CSS is using unsupported API/selector, and not guaranteed to work in future releases.

leluna commented 2 years ago

I would argue that multiple lines is rather rare with horizontally ordered radio buttons, which is the default. Thus, the default styling should primarily work nicely in this case. Maybe change the align-items to baseline for the vertical theme?

In grids where you have a visible horizontal guide (stripes or lines), it is quite prominent imo, and looks quite messy.

grafik

yuriy-fix commented 2 years ago

We will investigate if there is a way to align it in the grid without changing the baseline, but most probably it won't be changed (based on Jouni's comment).

rolfsmeds commented 2 years ago

Yeah I don't see us messing with the baseline alignment, but I think there's potential to fix this without doing that:

The two baseline-aligned elements are the label (green below) and the vaadin-radio-button-wrapper div (white below):

image

Both seem quite correctly aligned in themselves, but the radio shadow part div part="radio" that contains the button itself, is clearly not vertically centered inside the wrapper div.

Perhaps it's possible to adjust the vertical positioning of the radio part without touching the vertical alignment of its parent? WDYT @jouni ?

jouni commented 2 years ago

The baseline alignment of the radio button and the label has gotten broken at some point (I assume when the container was changed from a flex container to a grid container).

Here’s an example where I set --lumo-font-size-m: 24px;

Vaadin 23:

Screen Shot 2022-03-24 at 14 57 28

Vaadin 14:

Screen Shot 2022-03-24 at 14 57 10

The sizing of the radio button is intentionally now based on the --lumo-size-* properties (vs em in v14). But it should ideally still get nicely aligned vertically with the first line of label text.

jouni commented 2 years ago

I’ll try if there’s a way to make all these requirements happen at the same time.

web-padawan commented 2 years ago

@jouni We could probably revert grid container change as in introduced another problem: #3570

jouni commented 2 years ago

Some more observations:

yuriy-fix commented 5 months ago

@jouni, was it fixed with https://github.com/vaadin/web-components/pull/5288?