-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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(modal): Show/Hide when once prevented #2275
fix(modal): Show/Hide when once prevented #2275
Conversation
Codecov Report
@@ Coverage Diff @@
## dev #2275 +/- ##
=========================================
- Coverage 66.72% 66.52% -0.2%
=========================================
Files 162 162
Lines 3192 3167 -25
Branches 883 879 -4
=========================================
- Hits 2130 2107 -23
+ Misses 780 779 -1
+ Partials 282 281 -1
Continue to review full report at Codecov.
|
@tmorehouse Would be nice if we could solve this quickly :) |
Will just need to make sure |
if (showEvt.defaultPrevented || this.is_visible) { | ||
// Don't show if canceled | ||
this.is_opening = false |
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.
Good catch.
Just thinking about the root listeners added via the disable stacking PR that may be added, and as a just-in-case, remove any (with |
@tmorehouse Feel free to apply any changes 👍 |
Ok, will add the $off events in before destroy, just in case someone takes the component out of the page (I.e. route change, etc) |
Added in.. any thing else before merging? |
No, that should be it. |
getBoundingClientRects and getComputedStyle can't be tested in JSDOM environment, and just return empty strings and object values.
thanks for catching this! Always handy to have someone catch all the "troy-o's" |
Description of Pull Request:
#2270 broke showing/hiding of once prevented modals.
This PR fixes this.
PR checklist:
What kind of change does this PR introduce? (check at least one)
Does this PR introduce a breaking change? (check one)
If yes, please describe the impact:
The PR fulfills these requirements:
dev
branch, not themaster
branchfixes #xxxx[,#xxxx]
, where "xxxx" is the issue number)CHANGELOG
is generated from these messages.If new features/enhancement/fixes are added or changed:
package.json
for slot and event changes)If adding a new feature, or changing the functionality of an existing feature, the PR's description above includes: