-
-
Notifications
You must be signed in to change notification settings - Fork 29
feat(modal): add initialFocus prop for custom focus management #664
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: huyenltnguyen <[email protected]>
initialFocus
propThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot Address the feedback.
src/modal/modal.stories.tsx
Outdated
<p> | ||
When this modal opens, focus will be set to the "Custom Focus | ||
Target" button instead of the default close button. | ||
</p> | ||
<p> | ||
This is useful for forms or other scenarios where you want to direct | ||
the user's attention to a specific element immediately. | ||
</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The description should be added to the story description instead.
InitialFocusTemplate.parameters = {
docs: {
description: {
story:
"...",
},
},
}
You should:
- Change the modal body to contain some text and an input field.
- Place the initial focus on the input field.
- Use only two buttons on the modal: "Submit" and "Cancel".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated the story as requested. The description is now in story parameters, the modal contains a form with an input field that receives initial focus, and uses only Submit and Cancel buttons. Commit: c2cf9d2
src/modal/modal.test.tsx
Outdated
expect(customFocusButton).toHaveFocus(); | ||
}); | ||
|
||
it("should focus on the specified element even when close button is present and `initialFocus` is provided", async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The close button is always present, so we don't need this test case.
Instead, make sure there's a test for the scenario where initialFocus
is undefined. In that case, the close button should receive the initial focus.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed the redundant test case and added a test for when initialFocus
is undefined to ensure the close button receives focus by default. Commit: c2cf9d2
Co-authored-by: huyenltnguyen <[email protected]>
All feedback has been addressed. The story now demonstrates a realistic form scenario with input field focus, and the tests properly cover both defined and undefined Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
initialFocus
prop to ModalProps interface in types.tsinitialFocus
prop to Headless UI DialoginitialFocus
functionalityinitialFocus
usageImplementation Summary
Successfully added the
initialFocus
prop to the Modal component and addressed all review feedback. The implementation now includes:Changes Made
initialFocus
prop toModalProps
: AddedinitialFocus?: MutableRefObject<HTMLElement | null>
to the interfaceinitialFocus
prop to Headless UI's Dialog componentinitialFocus
is undefined to ensure close button gets focusValidation
The implementation now properly demonstrates the
initialFocus
prop in a realistic form scenario while maintaining full backwards compatibility.Fixes #15.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.