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

Fix bug with slots #61

Merged
merged 1 commit into from
Jun 29, 2017
Merged

Fix bug with slots #61

merged 1 commit into from
Jun 29, 2017

Conversation

rogozinds
Copy link
Contributor

@rogozinds rogozinds commented Jun 28, 2017

This change is Reviewable

@alvarezguille
Copy link
Member

test/vaadin-board_slot_test.html, line 63 at r1 (raw file):

Elements insdie board that have slots are not children of board.

There is a typo, and it's a bit confusing IMHO


Comments from Reviewable

@DiegoCardoso
Copy link
Contributor

Review status: 0 of 5 files reviewed at latest revision, 4 unresolved discussions, some commit checks broke.


test/board-element.html, line 8 at r1 (raw file):

:host {
 
}

It seems that this empty block is making build fails: block-no-empty and rule-empty-line-before rules.


test/outer.html, line 8 at r1 (raw file):

            :host {
                border-width: 3px;
                border-style:solid;

Build failing. Need to add a space after : because of declaration-colon-space-after rule.


test/vaadin-board_slot_test.html, line 58 at r1 (raw file):

            var child = children[i];

            assert.isAtLeast(child.getAttribute("style").indexOf("flex-basis: 25%"), 0,

This test is failing on my machine because flex-basis is 50%. Would it be because viewport is not large enough?


Comments from Reviewable

@rogozinds rogozinds force-pushed the fix_slot branch 2 times, most recently from c83a539 to ff4f797 Compare June 29, 2017 08:46
@rogozinds
Copy link
Contributor Author

Review status: 0 of 5 files reviewed at latest revision, 4 unresolved discussions.


test/outer.html, line 8 at r1 (raw file):

Previously, DiegoCardoso (Diego Cardoso) wrote…

Build failing. Need to add a space after : because of declaration-colon-space-after rule.

Done.


test/vaadin-board_slot_test.html, line 63 at r1 (raw file):

Previously, alvarezguille (Guille) wrote…

Elements insdie board that have slots are not children of board.

There is a typo, and it's a bit confusing IMHO

Done.


Comments from Reviewable

@rogozinds
Copy link
Contributor Author

Review status: 0 of 5 files reviewed at latest revision, 4 unresolved discussions.


test/board-element.html, line 8 at r1 (raw file):

Previously, DiegoCardoso (Diego Cardoso) wrote…

:host {
 
}

It seems that this empty block is making build fails: block-no-empty and rule-empty-line-before rules.

Done.


Comments from Reviewable

@rogozinds
Copy link
Contributor Author

Review status: 0 of 5 files reviewed at latest revision, 4 unresolved discussions.


test/vaadin-board_slot_test.html, line 58 at r1 (raw file):

Previously, DiegoCardoso (Diego Cardoso) wrote…

This test is failing on my machine because flex-basis is 50%. Would it be because viewport is not large enough?

Added fixed size of the board.


Comments from Reviewable

@rogozinds
Copy link
Contributor Author

Review status: 0 of 5 files reviewed at latest revision, 2 unresolved discussions.


test/vaadin-board_slot_test.html, line 58 at r1 (raw file):

Previously, rogozinds (rogozinds) wrote…

Added fixed size of the board.

Done.


Comments from Reviewable

@alvarezguille
Copy link
Member

Reviewed 4 of 5 files at r1, 3 of 3 files at r2.
Review status: all files reviewed at latest revision, 1 unresolved discussion.


Comments from Reviewable

@alvarezguille
Copy link
Member

:lgtm:


Comments from Reviewable

@DiegoCardoso
Copy link
Contributor

:lgtm:


Review status: all files reviewed at latest revision, all discussions resolved, some commit checks pending.


Comments from Reviewable

@DiegoCardoso DiegoCardoso merged commit 4ed4797 into master Jun 29, 2017
@DiegoCardoso DiegoCardoso deleted the fix_slot branch June 29, 2017 10:19
manolo pushed a commit to vaadin/flow-components that referenced this pull request Oct 3, 2020
manolo pushed a commit to vaadin/flow-components that referenced this pull request Oct 3, 2020
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.

3 participants