experiment: add dialog base styles and visual tests

[jouni]

Supported custom properties

Property	Description
--vaadin-dialog-background
--vaadin-dialog-border-width
--vaadin-dialog-border-color
--vaadin-dialog-border-radius
--vaadin-dialog-box-shadow
--vaadin-dialog-min-width	4em. Never larger than 100% (viewport width).
--vaadin-dialog-max-width	none
--vaadin-dialog-title-color
--vaadin-dialog-title-font-size	1em
--vaadin-dialog-title-font-weight	600
--vaadin-dialog-title-line-height	inherit
--vaadin-dialog-padding	The padding inside the header, content, and footer parts.
--vaadin-dialog-toolbar-gap	The gap between items in the header and footer toolbars.

[web-padawan]

Rebased and force-pushed to work after some core styles refactoring on main branch.

[web-padawan]

For some reason in the dev page the overlay collapses when toggling setting width: 280px (works fine with 300px):

[jouni]

For some reason in the dev page the overlay collapses when toggling setting width: 280px (works fine with 300px):

Thanks. Actually, that checkbox sets the height, I just forgot to update the label when I copied it. And setting the height of the dialog triggers the size container and size containment, which ends up collapsing the width.

I can fix this if I remove this if statement here, and always set the original bounds when resize is initiated:

web-components/packages/dialog/src/vaadin-dialog-resizable-mixin.js

Lines 75 to 77 in f2fec4a

	if (this.$.overlay.$.overlay.style.position !== 'absolute' || this.width || this.height) {
	this.$.overlay.setBounds(this._originalBounds);
	}

I’m not sure what this optimization accomplishes.

[jouni]

setting the height of the dialog triggers the size container and size containment, which ends up collapsing the width.

For the record, I ended up removing this feature (2a9aedd), as it’s likely unexpected for users/designers/developers. It’s probably better to leave this type of behavior for app-specific styles. Alternatively, we should flesh out the feature properly, how it could behave more robustly.

[web-padawan]

I’m not sure what this optimization accomplishes.

Added back in vaadin/vaadin-dialog@37a460c - apparently it was supposed to be only called on first drag / first resize.

[jouni]

apparently it was supposed to be only called on first drag / first resize

Right. Would be great to find out what the original issue was. I didn't notice any immediate downsides, but who knows, might be some weird edge case.

[jouni]

But regardless of the original issue, the size of the dialog should become fixed when the users drags it. Also, the size can become fixed in both directions when the user resizes either the width or the height.

[web-padawan]

Would be great to find out what the original issue was. I didn't notice any immediate downsides

Checked the review comments and couldn't find any issue that was referring to this change specifically 🤷‍♂️
IMO we can remove these checks indeed, at least based on the code it's unlikely to cause any problems.

[jouni]

@web-padawan, any idea why the visual tests fail?

[web-padawan]

Looks like I handled rebase incorrectly, pushed a fix to import overlay base styles from the correct location.

[web-padawan]

Let's consider reviewing #9486 and then rebasing this PR once that one is merged.

[web-padawan]

One more thing which is demo-page specific: on main branch, when you open a dialog, then drag it, close and reopen, it will use the previous position. In the new dev page, every button click opens another dialog, which isn't immediately obvious. Not sure if we should spend time changing this.

[web-padawan]

LGTM overall, but maybe someone else from the team should also review before merging.

[sonarqubecloud]

Quality Gate passed

Issues
5 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud