Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature #9357 - addStyleNames(...) #9537

Merged
merged 3 commits into from Jun 29, 2017
Merged

Feature #9357 - addStyleNames(...) #9537

merged 3 commits into from Jun 29, 2017

Conversation

appreciated
Copy link
Contributor

@appreciated appreciated commented Jun 14, 2017

#9357

Add the following functions to Abstract Component and the respective Interface Component:

  • addStyleNames(String... styles)
  • removeStyleNames(String... styles)

This change is Reviewable

Add the following functions to Abstract Component and the respective Interface Component:
- addStyleNames(String... styles)
- removeStyleNames(String... styles)
@CLAassistant
Copy link

CLAassistant commented Jun 14, 2017

CLA assistant check
All committers have signed the CLA.

@tsuoanttila
Copy link
Contributor

Reviewed 2 of 2 files at r1.
Review status: all files reviewed at latest revision, 3 unresolved discussions.


server/src/main/java/com/vaadin/ui/Component.java, line 181 at r1 (raw file):

     * @param styles
     *            the new style to be added to the component
     * @see #getStyleName()

Link to addStyleName for a more complete description of the feature


server/src/main/java/com/vaadin/ui/Component.java, line 185 at r1 (raw file):

     * @see #removeStyleName(String)
     */
    public void addStyleNames(String ... styles);

Since this is aimed for Vaadin 8, you should use Interface default methods from Java 8. This change as it is right now will break any custom implementation of Component that does not extend AbstractComponent.


server/src/main/java/com/vaadin/ui/Component.java, line 216 at r1 (raw file):

     * @see #addStyleName(String)
     */
    public void removeStyleNames(String ... styles);

see above comment about default method.


Comments from Reviewable

@appreciated appreciated changed the title Adds the required feature mentioned in Issue #9357 (https://github.co… Adds the required feature mentioned in Issue #9357 Jun 15, 2017
- removed implementation from AbstractComponent
- added implementation as default method to Component
@appreciated
Copy link
Contributor Author

You are right, I wasn't aware of possible custom implementations of Component.
I now added the implementations as default methods to Component.

- added @see to addStyleNames and removeStyleNames
@appreciated
Copy link
Contributor Author

Review status: 0 of 1 files reviewed at latest revision, 3 unresolved discussions.


server/src/main/java/com/vaadin/ui/Component.java, line 181 at r1 (raw file):

Previously, tsuoanttila (Teemu Suo-Anttila) wrote…

Link to addStyleName for a more complete description of the feature

Done.


server/src/main/java/com/vaadin/ui/Component.java, line 185 at r1 (raw file):

Previously, tsuoanttila (Teemu Suo-Anttila) wrote…

Since this is aimed for Vaadin 8, you should use Interface default methods from Java 8. This change as it is right now will break any custom implementation of Component that does not extend AbstractComponent.

Done.


server/src/main/java/com/vaadin/ui/Component.java, line 216 at r1 (raw file):

Previously, tsuoanttila (Teemu Suo-Anttila) wrote…

see above comment about default method.

Done.


Comments from Reviewable

@appreciated appreciated changed the title Adds the required feature mentioned in Issue #9357 Feature #9357 - addStyleNames(...) Jun 19, 2017
@hesara
Copy link
Contributor

hesara commented Jun 28, 2017

:lgtm: - dismissed Teemu's addressed review comments as he is on vacation


Reviewed 1 of 1 files at r3.
Review status: all files reviewed at latest revision, 3 unresolved discussions.


Comments from Reviewable

@elmot
Copy link
Contributor

elmot commented Jun 29, 2017

:lgtm:


Review status: all files reviewed at latest revision, 3 unresolved discussions.


Comments from Reviewable

@elmot elmot merged commit 415bdf9 into vaadin:master Jun 29, 2017
@elmot elmot added this to the 8.1.0.rc1 milestone Jun 29, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants