rusackas commented on PR #36306: URL: https://github.com/apache/superset/pull/36306#issuecomment-3613543674
Heya... just a few outstanding questions: * Any reason to remove `selected: undefined` from `expectedThemeProps`? I think that prop is still being set? * Is that removal (or something else) the reason we now use `expect.objectContaining` in the subsequent test? That's a bit looser, and may mask regressions. * I see now that All/Inv buttons are still there, so I'm not sure why they're being stripped from the test. Otherwise, I think we're about there... the basic fix looks good, just trying to make sense of the other changes and if/why they're necessary :D Thanks again for seeing this through! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
