-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Dialog scroll-lock bugfix #2120
Dialog scroll-lock bugfix #2120
Conversation
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.
|
Codecov Report
@@ Coverage Diff @@
## master #2120 +/- ##
==========================================
+ Coverage 98.48% 99.43% +0.94%
==========================================
Files 98 84 -14
Lines 4230 3720 -510
Branches 537 486 -51
==========================================
- Hits 4166 3699 -467
+ Misses 64 21 -43
Continue to review full report at Codecov.
|
I signed it! |
CLAs look good, thanks! |
@tompng Thanks for submitting this PR. Is there a reason that |
89a70c1
to
7150bd3
Compare
I agree, fixed. Little changes is that, scrolling is enabled before closing animation starts(originally, it was enabled after animation ends), but I think it's not a problem. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks for updating this.
When calling dialog.destroy() soon after dialog.close(), the whole window gets scroll-locked.
(when destroy() is called before the close transition ends)
this.enableScroll_()
cannnot placed insideif(this.isOpen_){}
because
this.close()
(it turnsthis.isOpen_ = false;
) does notenableScroll_()
other possibilities