search

vaadin / vaadin-button-flow

Vaadin Flow Java API for vaadin/vaadin-button Web Component
https://vaadin.com/components/vaadin-button
Other
0 stars 8 forks source link

Regression: icons always have prefix / suffix slot #140

Closed web-padawan closed 5 years ago

web-padawan commented 5 years ago

When using the icon-only button, as a developer I expect theme="icon" to be set.

However, after https://github.com/vaadin/vaadin-button-flow/pull/133 was merged, the behaviour has been changed, so that updateIconSlot always sets prefix / suffix.

The suggested fix is to use the same check as in updateThemeAttribute method:

https://github.com/vaadin/vaadin-button-flow/blob/93bba4217cb3b65b7fb6659511f90f5f04f8d80b/vaadin-button-flow/src/main/java/com/vaadin/flow/component/button/Button.java#L387

In that case, the slot should be left as default one (no slot attribute).

Discovered by @anezthes.

web-padawan commented 5 years ago

After taking a closer look, I think this can be closed as won't fix. We have a demo in the web component showing that icon theme works with prefix / suffix.

The actual issue in the Slack discussion was how icon theme affected tertiary-inline. Feel free to close unless we want to expose public API for handling slots.

Legioth commented 5 years ago

Feel free to close unless we want to expose public API for handling slots.

I'll take the bait.

If there is some use case where the user would need to do something related to the slots, then we should design an API accordingly instead of just adding a generic API. If one really wants to adjust the slots, then they can use getElement(). The slots themselves are an implementation detail from the perspective of the Java API.