Skip to content
This repository was archived by the owner on Jan 13, 2025. It is now read-only.

fix(drawer): Page content can be scrolled even if drawer is open #807

Merged
merged 1 commit into from
Jun 12, 2017

Conversation

vinhlh
Copy link
Contributor

@vinhlh vinhlh commented Jun 9, 2017

BREAKING CHANGE: Adapter API for temporary drawers contains two new methods: addBodyClass and removeBodyClass.

Closes #777

cc @amsheehan

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If your company signed a CLA, they designated a Point of Contact who decides which employees are authorized to participate. You may need to contact the Point of Contact for your company and ask to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the project maintainer to go/cla#troubleshoot.
  • In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again.

@codecov-io
Copy link

codecov-io commented Jun 9, 2017

Codecov Report

Merging #807 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #807      +/-   ##
==========================================
+ Coverage   99.93%   99.93%   +<.01%     
==========================================
  Files          68       68              
  Lines        3119     3134      +15     
  Branches      383      384       +1     
==========================================
+ Hits         3117     3132      +15     
  Misses          2        2
Impacted Files Coverage Δ
packages/mdc-drawer/temporary/constants.js 100% <ø> (ø) ⬆️
packages/mdc-drawer/temporary/index.js 100% <100%> (ø) ⬆️
packages/mdc-drawer/slidable/foundation.js 100% <100%> (ø) ⬆️
packages/mdc-drawer/temporary/foundation.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dec20ab...f5b41e9. Read the comment docs.

@googlebot
Copy link

CLAs look good, thanks!

Copy link
Contributor

@touficbatache touficbatache left a comment

Choose a reason for hiding this comment

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

Seems good!

Copy link
Contributor

@traviskaufman traviskaufman left a comment

Choose a reason for hiding this comment

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

Thank you! Just a few nits but overall looks good and seems to work nicely. Once the nits are addressed I'll merge it.

@@ -322,3 +322,18 @@ test('adapter#isDrawer returns false for a non-drawer element', () => {
const {root, component} = setupTest();
assert.isNotOk(component.getDefaultFoundation().adapter_.isDrawer(root));
});

test('adapter#addBodyClass adds a class to the body, locking the background scroll', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

techincally all addBodyClass/removeBodyClass does is add/remove a class to the body (respectively). It's up to the class itself to determine the stylistic behavior. So maybe for these two test descriptions just say adds a class to [/ removes a class from] the body.

BREAKING CHANGE: Adapter API for temporary drawers contains two new methods: `addBodyClass` and
`removeBodyClass`.

Closes material-components#777
@touficbatache
Copy link
Contributor

@traviskaufman The Travis-CI build passed

@traviskaufman traviskaufman merged commit 8686d85 into material-components:master Jun 12, 2017
@torstahl
Copy link

Hello

this works great on normal Browser.
I made a cordova app for Android and IOS and and there it is still possible to scroll the body content when the drawer ist open.
The body get the class mdc-drawer-scroll-lock when open. But still scrolling is possible.
On Android Webview and Ios Safari

BR/Torsten
screenshot_20171111_101917

@torstahl
Copy link

Hello again
i add touch-action: none; to .mdc-drawer-scroll-lock
BR/Torsten

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.

6 participants