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
443 stars 83 forks source link

[dialog] Allow custom role-attribute for dialog #4977

Closed knoobie closed 1 month ago

knoobie commented 1 year ago

What is the problem?

Allow overriding of role=dialog from the dialog/overlay to e.g. change it to alertdialog - related to https://github.com/vaadin/web-components/issues/4975

This is possible with other components ;) https://github.com/vaadin/web-components/issues/4782

https://github.com/vaadin/web-components/blob/be240c8459b622f3deae45c03ff3b8fd0449b49a/packages/dialog/src/vaadin-dialog.js#L605-L609

Browsers

Screen Readers

knoobie commented 1 year ago

@web-padawan Can I get this also? ;) This would match with other components and your current 24.1 theme about dialog changes. Reasoning: The confirm dialog isn't customizeable enough for me, that's why I'm relying on the default dialog.

web-padawan commented 1 year ago

@web-padawan Can I get this also?

Thanks for reminding about this issue. It was not included to Vaadin 24.1 scope. We need to come up with API for it, and the beta1 is already out, which means we can't add any new features at this point. So I'm afraid it has to be postponed.

knoobie commented 1 year ago

I'm personally not looking for a new API, but now as you say it.. it's different from the linked issue with button - because the role isn't on the dialog like done with button, instead it's on the "nested" overlay element. Postponed 👍

web-padawan commented 1 year ago

it's different from the linked issue with button - because the role isn't on the dialog like done with button, instead it's on the "nested" overlay element.

Yes, this is exactly why we need to provide a property on the host element for configuring it.

rolfsmeds commented 1 month ago

@web-padawan I think we recently decided to go with an overlayRole property for vaadin-popover, so a corresponding API for Dialog would be obvious. I presume the Flow equivalent is setOverlayRole?

web-padawan commented 1 month ago

Yes, we added setOverlayRole to the Flow counterpart. I agree it makes sense to add this API to vaadin-dialog too.

web-padawan commented 1 month ago

Fixed by https://github.com/vaadin/web-components/pull/7582 and https://github.com/vaadin/flow-components/pull/6473. This feature will be available in 24.5 stable release.