fix(top-app-bar): Fix JS error when navigation icon is not present. #2751
Conversation
…n icon is not present. Fixes Top App Bar's initialize method to add ripple to nav icon only when it is present.
Codecov Report
@@ Coverage Diff @@
## master #2751 +/- ##
==========================================
+ Coverage 98.48% 98.48% +<.01%
==========================================
Files 98 98
Lines 4230 4231 +1
Branches 537 538 +1
==========================================
+ Hits 4166 4167 +1
Misses 64 64
Continue to review full report at Codecov.
|
|
This is something we should be able to verify with a new unit test (make sure it fails without the fix and passes with it).
Edit: I think it wasn't failing due to the use of |
…ipple factory. Also updated existing test case to use stubs instead of asserting private properties.
|
Yes, the use of FakeRipple in unit test suppresses this bug. I've added a new test case to cover this case also updated other test case to make use of stubs instead of asserting on private properties. PTAL. |
| td.verify(rippleFactory(td.matchers.anything()), {times: totalIcons}); | ||
| }); | ||
|
|
||
| test('constructor instantiates icon ripples except for nav icon', () => { |
There was a problem hiding this comment.
Nit: Rephrase to 'constructor does not instantiate ripple for nav icon when not present'
| const {root} = setupTest(/** removeIcon */ false, rippleFactory); | ||
|
|
||
| const totalIcons = getIconsCount(root); | ||
| td.verify(rippleFactory(td.matchers.anything()), {times: totalIcons}); |
There was a problem hiding this comment.
This test and the following test are causing warnings in the test runner and CI. Line 99 is mocking this object, and then it's testing that the mocked object got called.
WARN: 'Warning: testdouble.js - td.verify - test double was both stubbed and verified with arguments (anything()), which is redundant and probably unnecessary. (see: https://github.com/testdouble/testdouble.js/blob/master/docs/B-frequently-asked-questions.md#why-shouldnt-i-call-both-tdwhen-and-tdverify-for-a-single-interaction-with-a-test-double )'
There was a problem hiding this comment.
Whoops, good catch. I suppose since we're stubbing the function, inside it we can increment a variable that's local to the test function, and just assert that it has the value we expect afterwards?
Fixes Top App Bar's initialize method to add ripple to nav icon only when it is present.
Fixes #2721