It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group.

Hey @BenjiFarquhar do you plan to continue working on this change?

Hey @BenjiFarquhar do you plan to continue working on this change?

Hey @Piinks, I'd appreciate if someone can add a test for this. I may be able to do it soon if there are no takers.

Yeah, if you'd be able to add the test at some point that'd be great!

I believe generally, test changes are up to the PR author. If you're not able to get around to it, we can close this pull request and keep #155207 open, and we'll see if anyone decides to pick it up!

@nate-thegrate Should be all good now. I think the test failure was saying the expansion panel header is in an enabled state, and the semantics represent this with the isEnabled flag. Adding isEnabled: true to the expansion panel header semantics test made it happy. Disabling the IconButton must have caused the isEnabled flag to be false in the semantics tree (which would be misleading to assistive technologies since the whole header is enabled). Let me know if you want any changes or are not sure about my test fix.

CC: @Piinks

My understanding is the approach in #147098 was taken to avoid Google testing failures? But it has undesired side effects and this PR is reverting it?

@victorsanni yeah, original issue was it disabled the IconButton when canTapOnHeader is true to give button tap control over to the whole ExpansionPanel header, and the fix from #147098 was to address the disabled icon color showing, by changing the disabled color to be the enabled color. but still, when the IconButton was disabled, it broke the mouse pointer when hovering on the expansion panel header, specifically on the IconButton it became the triangle shape, and on the rest of the header it was the hand to express a tappable area. Disabled button changes the mouse pointer to express that it is not tappable. Disabled button also removed isEnabled from the Semantics and the semantics test was not including isEnabled, since it was technically disabled with the old way, so my change broke a semantics test, and i fixed it by adding isEnabled to the test. So I have fixed it here in a different way that fixes all of those things.

@BenjiFarquhar so basically this PR allows the icon to look enabled (i.e darker) instead of disabled (i.e lighter) when canTapOnHeader is set to true?

@victorsanni Yes. When canTapOnHeader is true, the IconButton now behaves merely as an Icon (appearing enabled), signalling to the user that the header can expand/collapse, and doesn't interfere with hovering effects on the expansion panel header.

LGTM pending review from @TahaTesser.