fix(datepicker): focus on multiple instances (#3494) #3549
Conversation
|
FYI tests are failing because of an unrelated issue. I just pushed the fix to the master |
gpolychronis
force-pushed
the
datepicker/focus/multipleInstancesBug
branch
from
437a526
to
27da357
Compare
Codecov Report
@@ Coverage Diff @@
## master #3549 +/- ##
==========================================
+ Coverage 91.67% 91.67% +<.01%
==========================================
Files 100 100
Lines 2882 2883 +1
Branches 532 532
==========================================
+ Hits 2642 2643 +1
Misses 183 183
Partials 57 57
Continue to review full report at Codecov.
|
gpolychronis
force-pushed
the
datepicker/focus/multipleInstancesBug
branch
from
27da357
to
18e84c8
Compare
|
Added tests. Also, it seems to fix the performance issue of #3396 |
There was a problem hiding this comment.
Hey, @gpolychronisamadeus.
LGTM, thanks for adding the test
Left three small comments, will merge after it is fixed right away, so you could rebase your main PR on this.
Cheers,
Max
| await page.expectActive(new Date(2016, 7, 8), 0); | ||
| await page.getDayElement(new Date(2016, 7, 1), 1).click(); | ||
| await sendKey(Key.ARROW_DOWN); | ||
| await page.expectActive(new Date(2016, 7, 8), 1); |
There was a problem hiding this comment.
could you please check here that the day is actually focused? there is an expectFocused() utility for this
It still can be active, but not focused
e2e-app/src/app/datepicker/multiple/datepicker-multiple.component.html
Outdated
Show resolved
Hide resolved
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
gpolychronis
force-pushed
the
datepicker/focus/multipleInstancesBug
branch
from
18e84c8
to
629df9b
Compare
Before submitting a pull request, please make sure you have at least performed the following: