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

Adding demo for hidden splitter. #76

Merged
merged 1 commit into from
Jul 27, 2017
Merged

Adding demo for hidden splitter. #76

merged 1 commit into from
Jul 27, 2017

Conversation

manolo
Copy link
Member

@manolo manolo commented Jul 27, 2017

Fixes #62


This change is Reviewable

@limonte
Copy link
Contributor

limonte commented Jul 27, 2017

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


demo/theming.html, line 60 at r1 (raw file):

        <dom-bind>
          <template is="dom-bind">
            <label><input aria-label="Select Row" type="checkbox" checked="{{_hideLeft::change}}">Hide Left Section</label>

vaadin-checkbox will look nicer here:

<vaadin-checkbox checked="{{_hideLeft::change}}" aria-label="Select Row">
  Hide Left Section
</vaadin-checkbox>

Comments from Reviewable

@manolo
Copy link
Member Author

manolo commented Jul 27, 2017

Review status: 0 of 1 files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.


demo/theming.html, line 60 at r1 (raw file):

Previously, limonte (Limon Monte) wrote…

vaadin-checkbox will look nicer here:

<vaadin-checkbox checked="{{_hideLeft::change}}" aria-label="Select Row">
  Hide Left Section
</vaadin-checkbox>

I’m not sure about adding the vaadin-checkbox dev-dependency in the split-layout just for a demo. Also depending on an unfinished component sounds not ok. I agree that the demo could be nicer though.
Also the default theme for vaadin-checkbox is a native look, we would need to depend on themes, but we tried to avoid that loop dependency.


Comments from Reviewable

@limonte
Copy link
Contributor

limonte commented Jul 27, 2017

:lgtm:


Review status: 0 of 1 files reviewed at latest revision, all discussions resolved, some commit checks failed.


Comments from Reviewable

@limonte
Copy link
Contributor

limonte commented Jul 27, 2017

Reviewed 1 of 1 files at r1, 1 of 1 files at r2.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@peancored
Copy link
Contributor

:lgtm:


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


Comments from Reviewable

@peancored peancored merged commit 22e8e02 into master Jul 27, 2017
@peancored peancored deleted the demo/hide-splitter branch July 27, 2017 08:43
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

3 participants