search

vaadin / vaadin-button-flow

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

Use prefix and suffix slots for icons #133

Closed pekam closed 5 years ago

pekam commented 5 years ago

Fix #37

As a consequence, the order of child nodes no more respects the iconAfterText property (it does not need to).

Also, text content is no more wrapped inside a when using both icon and text content.

It's not clear whether this is a breaking change or a bug fix, as users might have applied styling which depends on the old behavior.

This change is Reviewable

vaadin-bot commented 5 years ago

SonarQube analysis reported 4 issues

Note: The following issues were found on lines that were not modified in the pull request. Because these issues can't be reported as line comments, they are summarized here:

  1. MAJOR Button.java#L50: Remove this unused "disableListener" private field. rule
  2. MINOR Button.java#L267: Remove this use of "callFunction"; it is deprecated. rule
  3. MINOR Button.java#L345: Remove this use of "executeJavaScript"; it is deprecated. rule
  4. MINOR Button.java#L419: Remove this use of "executeJavaScript"; it is deprecated. rule
pekam commented 5 years ago

Actually this could be done in a less-breaking way by simply setting the slot attribute and keeping all the reordering and span-wrapping logic like before.

I'd still like to remove all that unnecessary and complex stuff now when we are releasing a new major for an LTS platform. With this PR, Button should finally have the same structure as the web component, without any extra spans.

The less-breaking fix can be considered to be made for the V10 version, or maybe not since getting the correct slot spacing etc. from the web component might still break application styles for users who depend on the old behavior.