Skip to content
This repository has been archived by the owner on Oct 8, 2020. It is now read-only.

Remove theme variants classes. #71

Merged
merged 1 commit into from
Jul 18, 2018
Merged

Conversation

SomeoneToIgnore
Copy link
Contributor

@SomeoneToIgnore SomeoneToIgnore commented Jul 18, 2018

Since both layouts do not extend the generated classes and already have an api that covers all the variants,
it makes no sense to have variants classes here.


This change is Reviewable

Copy link
Contributor

@ZheSun88 ZheSun88 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 5 of 5 files at r1.
Reviewable status: 0 of 1 LGTMs obtained


src/test/java/com/vaadin/flow/component/orderedlayout/tests/VerticalLayoutViewIT.java, line 44 at r1 (raw file):

                vLayout.getAttribute("theme").contains("spacing"));
        Assert.assertTrue(
                "By default layout should contain spacing theme in 'theme' attribute",

should contain margin here.

Since both layouts do not extend the generated classes and already have an api that covers all the variants,
it makes no sense to have variants classes here.
@SomeoneToIgnore
Copy link
Contributor Author


src/test/java/com/vaadin/flow/component/orderedlayout/tests/VerticalLayoutViewIT.java, line 44 at r1 (raw file):

Previously, ZheSun88 (Sun Zhe) wrote…

should contain margin here.

Fixed.

@SomeoneToIgnore SomeoneToIgnore force-pushed the kb/remove-variants-classes branch from 4b9c473 to d622718 Compare July 18, 2018 09:00
Copy link
Contributor

@ZheSun88 ZheSun88 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 1 files at r2.
Reviewable status: 0 of 1 LGTMs obtained

Copy link
Contributor

@ZheSun88 ZheSun88 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewable status: 0 of 1 LGTMs obtained, and 1 stale

@SomeoneToIgnore SomeoneToIgnore merged commit 0c4919f into master Jul 18, 2018
@SomeoneToIgnore SomeoneToIgnore deleted the kb/remove-variants-classes branch July 18, 2018 11:05
@gilberto-torrezan gilberto-torrezan added this to the 1.1.0.alpha1 milestone Jul 18, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants