[Accordion] Fix IconButtonProps spreading logic #21850
Conversation
oliviertassinari
added
the
component: accordion
oliviertassinari
changed the title
packages/material-ui/src/AccordionSummary/AccordionSummary.test.js
Outdated
Show resolved
Hide resolved
| @@ -99,6 +111,17 @@ describe('<AccordionSummary />', () => { | |||
| expect(button).not.to.have.class(classes.focused); | |||
| }); | |||
|
|
|||
| it('should fire onBlur when the button blurs', () => { | |||
There was a problem hiding this comment.
I don't understand why it's green in my env
There was a problem hiding this comment.
Ohhh, it's because I'm running the tests in watch mode! I still haven't got used to mochajs/mocha#4347.
There was a problem hiding this comment.
It passes, but I think the idea is that we're asserting on the result of the field receiving focus and blurring and we want to ensure that the updates are completed prior to assertion.
There are a few other places in the codebase that are inconsistent about this, so I didn't wrap it here (e.g. Select.test.js, , but I probably should have. Sorry about that.
@eps1lon what is the difference between this (in blur should unset unfocused state):
act(() => {
button.focus();
});
act(() => {
button.blur();
});
...and wrapping them with one act? Is it that we're allowing focus to update before we blur?
…ops are provided Ensure that `expanded` and `IconButtonProps.className` are applied. Added `onBlur` test to improve coverage of this component.
|
@kgregory Thanks, it's great to not only fix the bug in question but to actually be a good boy-scout, leaving the place in better shape than when entering. |
|
@oliviertassinari thanks. Sorry about missing that |
|
@kgregory I think that it's simply because your fork next branch is from an old version (~20 days), your build was green before I rebased it. Try pulling the latest changes :) |
|
@oliviertassinari oh that's embarrassing. Will do. |
The
expandedclass was being replaced whenIconButtonPropswas provided with aclassName. To fix this, we revisit the application ofclassNametoIconButton. We will continue to conditionally applyexpanded, but now we'll combine it withclassNamefromIconButtonProps, which now defaults to an empty object.Added
onBlurtest to improve coverage of this component.Regenerated docs.
Closes #21744