Skip to content

Commit 8b713e2

Browse files
authored
Adds a bugfix for ModalBackdrop when dialogs are conditionally rendered (#2867)
## Summary: When trying to refactor conditional modals in frontend, there were some internal test failures pointing to ModalBackdrop as the cause. This PR exists to address errors in frontend unit tests with WB changes for conditional dialog rendering. Issue: WB-2103 ## Test plan: Author: marcysutton Reviewers: marcysutton, beaesguerra Required Reviewers: Approved By: beaesguerra Checks: ✅ 12 checks were successful, ⏭️ 3 checks have been skipped Pull Request URL: #2867
1 parent 415dd5a commit 8b713e2

File tree

4 files changed

+307
-100
lines changed

4 files changed

+307
-100
lines changed

.changeset/weak-needles-fail.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@khanacademy/wonder-blocks-modal": patch
3+
---
4+
5+
Adds a bugfix for ModalBackdrop when dialogs are conditionally rendered

__docs__/wonder-blocks-modal/modal-launcher.stories.tsx

Lines changed: 65 additions & 93 deletions
Original file line numberDiff line numberDiff line change
@@ -779,125 +779,97 @@ const styles = StyleSheet.create({
779779
});
780780

781781
/**
782-
* This story helps debug focus management with controlled modals.
783-
It specifically tests the case where a modal starts with opened=true and
784-
needs to return focus to the last focused element when closed.
785-
786-
Because the wrapped ModalLauncher is unmounted on close, it needs special
787-
focus management logic to return focus back to the main page.
782+
* This story demonstrates using a single controlled ModalLauncher with
783+
* conditional dialog content. Different buttons trigger different modal
784+
* content, and focus management is handled correctly when the modal closes.
785+
*
786+
* This pattern is useful when you need to show different dialogs based on
787+
* user interaction, but want to manage them through a single ModalLauncher
788+
* instance.
788789
*/
789-
export const ControlledModalFocusTest: StoryComponentType = () => {
790-
const [opened, setOpened] = React.useState(false);
791-
const [showModal, setShowModal] = React.useState(false);
790+
export const ConditionalDialogsWithFocusManagement: StoryComponentType = () => {
791+
const [openedModal, setOpenedModal] = React.useState<
792+
"REGULAR" | "WRAPPED" | null
793+
>(null);
792794

793-
// This button simulates what we're interacting with before the modal appears
794-
const buttonRef = React.useRef<HTMLButtonElement>(null);
795+
const handleClose = () => {
796+
setOpenedModal(null);
797+
};
798+
799+
const conditionalDialog = ({closeModal}: {closeModal: () => void}) => {
800+
if (openedModal === "REGULAR") {
801+
return (
802+
<OnePaneDialog
803+
title="Regular Modal"
804+
content={<View>This is a regular modal</View>}
805+
footer={
806+
<Button onClick={() => closeModal()}>
807+
Close Modal
808+
</Button>
809+
}
810+
/>
811+
);
812+
}
795813

796-
React.useEffect(() => {
797-
// Focus the button when component mounts
798-
if (buttonRef.current) {
799-
buttonRef.current.focus();
814+
if (openedModal === "WRAPPED") {
815+
return (
816+
<OnePaneDialog
817+
title="Alternative Modal"
818+
content={
819+
<View>
820+
This is an alternative modal with different content
821+
</View>
822+
}
823+
footer={<Button onClick={closeModal}>Close Modal</Button>}
824+
/>
825+
);
800826
}
801-
}, []);
802827

803-
const handleShowWrappedModal = () => {
804-
setShowModal(true);
828+
// Fallback (should not be reached)
829+
return null;
805830
};
806831

807832
return (
808833
<View style={styles.storyContainer}>
809834
<BodyText>
810-
This story reproduces the test case for controlled modals and
811-
focus management. Steps to test: 1. Click &quot;Focus me
812-
first&quot; button 2. Click &quot;Show wrapped modal&quot;
813-
button 3. Close the modal using the close button 4. Focus should
814-
return to &quot;Focus me first&quot; button
835+
This story demonstrates conditional dialogs within a single
836+
ModalLauncher. Click either button to open different modal
837+
content. When the modal closes, focus returns to the triggering
838+
button using the `closedFocusId` prop.
815839
</BodyText>
816840

817841
<View style={styles.buttonRow}>
818842
<Button
819-
ref={buttonRef}
820-
onClick={() => setOpened(true)}
821-
testId="focus-target"
843+
onClick={() => setOpenedModal("REGULAR")}
844+
testId="regular-modal-trigger"
822845
>
823-
Focus me first
846+
Open Regular Modal
824847
</Button>
825848

826-
<Button onClick={handleShowWrappedModal} id="second-target">
827-
Show wrapped modal
849+
<Button
850+
onClick={() => setOpenedModal("WRAPPED")}
851+
id="alternative-modal-trigger"
852+
>
853+
Open Alternative Modal
828854
</Button>
829855
</View>
830856

831-
{/* Regular modal to verify focus behavior */}
857+
{/* Single ModalLauncher with conditional dialogs */}
832858
<ModalLauncher
833-
modal={({closeModal}) => (
834-
<OnePaneDialog
835-
title="Regular Modal"
836-
content={<View>This is a regular modal</View>}
837-
footer={
838-
<Button onClick={() => closeModal()}>
839-
Close Modal
840-
</Button>
841-
}
842-
/>
843-
)}
844-
opened={opened}
845-
onClose={() => {
846-
setOpened(false);
847-
}}
859+
opened={openedModal !== null}
860+
onClose={handleClose}
861+
closedFocusId={
862+
openedModal === "WRAPPED"
863+
? "alternative-modal-trigger"
864+
: undefined
865+
}
866+
modal={conditionalDialog}
848867
/>
849-
850-
{/* Wrapped modal that starts opened=true */}
851-
{showModal && (
852-
<WrappedModalExample
853-
onClose={() => setShowModal(false)}
854-
returnFocusToId="second-target"
855-
/>
856-
)}
857868
</View>
858869
);
859870
};
860871

861-
ControlledModalFocusTest.parameters = {};
862-
863-
const WrappedModalExample = ({
864-
onClose,
865-
returnFocusToId,
866-
}: {
867-
onClose: () => void;
868-
returnFocusToId?: string;
869-
}) => {
870-
const isClosingRef = React.useRef(false);
871-
const handleCloseWithFocusReturn = () => {
872-
isClosingRef.current = true;
873-
874-
onClose();
875-
};
876-
React.useEffect(() => {
877-
return () => {
878-
// This cleanup function runs when the component unmounts
879-
if (isClosingRef.current && returnFocusToId) {
880-
const element = document.getElementById(returnFocusToId);
881-
if (element) {
882-
element.focus();
883-
}
884-
}
885-
};
886-
}, [returnFocusToId]);
887-
return (
888-
<ModalLauncher
889-
onClose={handleCloseWithFocusReturn}
890-
opened={true}
891-
modal={({closeModal}) => (
892-
<OnePaneDialog
893-
title="Wrapped Modal"
894-
content={<View>This modal starts with opened=true</View>}
895-
footer={<Button onClick={closeModal}>Close Modal</Button>}
896-
/>
897-
)}
898-
/>
899-
);
900-
};
872+
ConditionalDialogsWithFocusManagement.parameters = {};
901873

902874
/**
903875
* This example demonstrates how to use `ModalLauncher` to launch a modal that

0 commit comments

Comments
 (0)