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(material): put drawer over navbar shadow #121

Merged
merged 2 commits into from
Nov 13, 2019
Merged

fix(material): put drawer over navbar shadow #121

merged 2 commits into from
Nov 13, 2019

Conversation

AdrienC
Copy link

@AdrienC AdrienC commented Oct 1, 2019

This change is Reviewable

@AdrienC AdrienC changed the title fix(material): put drawer over navbar fix(material): put drawer over navbar shadow Oct 10, 2019
@web-padawan web-padawan self-assigned this Nov 7, 2019
@AdrienC
Copy link
Author

AdrienC commented Nov 7, 2019

Hi,
my drawer was the primary section (appLayout.setPrimarySection(AppLayout.Section.DRAWER)), so it was over the navbar.
Before the fix :
before_fix
After the fix :
after_fix
On the Material Navigation Drawer page, we can see there's no navbar left shadow over the drawer :
material

My first commit didn't work for all cases (drawer not the primary section), so I fixed it.

@web-padawan web-padawan changed the base branch from master to material-drawer-fix November 13, 2019 08:56
Copy link
Member

@web-padawan web-padawan 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: all files reviewed, 1 unresolved discussion (waiting on @DiegoCardoso)


theme/material/vaadin-app-layout-styles.html, line 22 at r1 (raw file):

Previously, DiegoCardoso (Diego Cardoso) wrote…

Thanks for the contribution.

Is this the desired behavior with this change?
image

If so, I believe that navbar should indeed drop its shadow over drawer. Take Material Components page as example:
image

Tested the latest change and this is now fixed.

Copy link
Member

@web-padawan web-padawan 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: all files reviewed, 1 unresolved discussion (waiting on @DiegoCardoso)

Copy link
Contributor

@DiegoCardoso DiegoCardoso 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: :shipit: complete! all files reviewed, all discussions resolved

Copy link
Member

@web-padawan web-padawan left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@web-padawan web-padawan merged commit f7fcd28 into vaadin:material-drawer-fix Nov 13, 2019
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