Make sure Dialogs do not render close button when onClose callback is not provided #1045
Conversation
| @@ -216,7 +215,7 @@ const Dialog = function Dialog({ | |||
| buttons={buttons} | |||
| fullWidthHeader={true} | |||
| icon={icon} | |||
| onClose={closeHandler} | |||
| onClose={isClosableDialog ? closeHandler : undefined} | |||
There was a problem hiding this comment.
I could just have done onClose ? closeHandler : undefined, but I feel starting with isClosableDialog explains better the "why".
| const wrapper = mount(<Dialog title="My dialog" />); | ||
| assert.isFalse(wrapper.find('CancelIcon').exists()); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
This test fails in main.
Codecov Report
@@ Coverage Diff @@
## main #1045 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 53 53
Lines 715 715
Branches 251 251
=========================================
Hits 715 715
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
|
||
| const closeHandler = useCallback(() => { | ||
| if (TransitionComponent) { | ||
| // When a TransitionComponent is provided, the actual "onClose" will be | ||
| // called by that component, once the "out" transition has finished | ||
| setTransitionComponentVisible(false); | ||
| } else { | ||
| onClose(); | ||
| onClose?.(); |
There was a problem hiding this comment.
Technically we know that onClose is present at this point, but TS wouldn't, I suppose. I'm fine leaving this as-is.
There was a problem hiding this comment.
That's actually a good point 🤔
I kind-of want to give this a thought and see if I can find a way to make TS know this is actually set at this point.
| @@ -216,7 +215,7 @@ const Dialog = function Dialog({ | |||
| buttons={buttons} | |||
| fullWidthHeader={true} | |||
| icon={icon} | |||
| onClose={closeHandler} | |||
| onClose={isClosableDialog ? closeHandler : undefined} | |||
| const wrapper = mount(<Dialog title="My dialog" />); | ||
| assert.isFalse(wrapper.find('CancelIcon').exists()); | ||
| }); | ||
| }); |
Closes #1044
This PR changes the logic in
Dialogto make sure anonClosecallback is not passed down to thePanel, if theDialogitself did not get one.