fix(datepicker): refine focus state checks #3530
Conversation
This adds additional condition to restrict focus in/out to instance of the datepicker by verifyng if focus event target and related targets are descendants of the datepicker element. This removes false results based only on the class name checks, which fail when focus is switched between datepicker instances. Closes ng-bootstrap#3494
Codecov Report
@@ Coverage Diff @@
## master #3530 +/- ##
=======================================
Coverage 91.39% 91.39%
=======================================
Files 96 96
Lines 2800 2800
Branches 517 517
=======================================
Hits 2559 2559
Misses 183 183
Partials 58 58
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Thanks @peterblazejewicz for the fix proposal. Seems that it should be fine!
Nonetheless, could you:
- create a 💪 end2end test for that
- make sure that there's no performance penalty overall by profiling before/after
| this._elementRef.nativeElement.contains(target as Node) && | ||
| this._elementRef.nativeElement.contains(relatedTarget as Node))), |
There was a problem hiding this comment.
Please create a local reference const {nativeElement} = this._elementRef; outside of the Observable chain.
|
@peterblazejewicz or actually even a unit test might be fine, I think, if you simulate |
|
Closing in favour of #3549 that is based on this commit + has tests |
This adds additional condition to restrict focus in/out to instance of
the datepicker by verifyng if focus event target and related targets are
descendants of the datepicker element. This removes false results based
only on the class name checks, which fail when focus is switched between
datepicker instances.
Closes #3494
/cc
@gpolychronisamadeus