feat(sidenav): configurable scroll prevent target #9338
Conversation
crisbeto
added
needs: review
| if (disableScrollTarget) { | ||
| disableScrollTarget = angular.element(disableScrollTarget); | ||
| } else { | ||
| $log.warn($mdUtil.supplant('mdSidenav: couldn\'t find element matching ' + |
There was a problem hiding this comment.
Perhaps we should convert all $log.warn() to $log.debug() since .debug() can be disabled globally with $logProvider.debugEnabled(false)
There was a problem hiding this comment.
I'm not sure we'd want that, because it could lead to cases where this might break in production, but the user wouldn't be able to see what went wrong.
There was a problem hiding this comment.
This is a breaking feature then... because the framework will complain using the default configuration (without a disabel scroll target).
There was a problem hiding this comment.
It won't complain. If it's not specified, it will default to the direct parent like it does at the moment.
There was a problem hiding this comment.
You are correct. I was confused with the code above... but the full code view shows that my concern is invalid. Thx.
* Adds the ability to select which element's scrolling will be disabled when a sidenav is open. This can be useful in the cases where the scrollable container isn't the direct parent of the sidenav. * Adds unit tests for the parent scroll prevention. Fixes angular#8634
crisbeto
force-pushed
the
8634/sidenav-prevent-scroll
branch
from
1779dce to
7d5912a
Compare
crisbeto
added
needs: review
ThomasBurleson
added
pr: merge ready
Fixes #8634