Skip to content
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

add new props for customizing internal Modal elements #1821

Merged
merged 5 commits into from
Feb 5, 2024

Conversation

mayank99
Copy link
Contributor

@mayank99 mayank99 commented Feb 5, 2024

Changes

Addresses second point of #1368 (comment)

Testing

Added unit tests for new props.

Docs

Added changesets.

@mayank99 mayank99 self-assigned this Feb 5, 2024
@mayank99 mayank99 marked this pull request as ready for review February 5, 2024 21:20
@mayank99 mayank99 requested review from a team as code owners February 5, 2024 21:20
@mayank99 mayank99 requested review from r100-stack and AdamMeza-Bentley and removed request for a team February 5, 2024 21:20
@@ -99,10 +114,11 @@ export const Modal = React.forwardRef((props, forwardedRef) => {
setFocus
ref={forwardedRef}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part is very annoying: ref and ...rest props are applied on different elements. I thought we fixed it in v3, but evidently not.

@mayank99 mayank99 changed the title allow customzing internal Modal elements allow customizing internal Modal elements Feb 5, 2024
@mayank99 mayank99 changed the title allow customizing internal Modal elements add new props for customizing internal Modal elements Feb 5, 2024
@mayank99 mayank99 merged commit 86e88b6 into main Feb 5, 2024
16 checks passed
@mayank99 mayank99 deleted the mayank/modal-props branch February 5, 2024 22:27
@imodeljs-admin imodeljs-admin mentioned this pull request Feb 5, 2024
@mayank99 mayank99 mentioned this pull request Feb 6, 2024
32 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants