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

fix(ComposedModal): Address several problems with selectorsFloatingMenus prop on ComposedModal #10003

Merged
merged 9 commits into from
Nov 8, 2021

Conversation

dcwarwick
Copy link
Contributor

Closes #10000

Changelog

Changed

  • packages/react/src/components/ComposedModal/ComposedModal.js -- fixed proptype, and remove prop from ...others that are passed through to the DOM.
  • packages/react/src/internal/wrapFocus.js -- default Carbon CSS selectors for floating menus/popus are added to prop value rather than replaced by it.

Testing / Reviewing

Build, tests, and storybook can help verify that this PR does not break existing usage. In particular, a story or test case with a menu/tooltip/date-picker inside a composed modal would be useful to verify that focus handling continues to work as expected.

I will also apply the changes locally to a build of carbon-design-system/ibm-products#1360 and verify that the issues that were problematic with that fix are resolved by these changes.

@dcwarwick dcwarwick requested a review from a team as a code owner October 31, 2021 16:15
@netlify
Copy link

netlify bot commented Oct 31, 2021

✔️ Deploy Preview for carbon-react-next ready!

🔨 Explore the source changes: a6695b9

🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-react-next/deploys/618952f64004980008636a4b

😎 Browse the preview: https://deploy-preview-10003--carbon-react-next.netlify.app

@netlify
Copy link

netlify bot commented Oct 31, 2021

✔️ Deploy Preview for carbon-elements ready!

🔨 Explore the source changes: a6695b9

🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-elements/deploys/618952f6712cfb0008fb1998

😎 Browse the preview: https://deploy-preview-10003--carbon-elements.netlify.app

@netlify
Copy link

netlify bot commented Oct 31, 2021

✔️ Deploy Preview for carbon-components-react ready!
Built without sensitive environment variables

🔨 Explore the source changes: a6695b9

🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-components-react/deploys/618952f673cf450008dc5e2f

😎 Browse the preview: https://deploy-preview-10003--carbon-components-react.netlify.app

@dcwarwick dcwarwick requested a review from a team as a code owner November 1, 2021 01:00
Copy link
Member

@tay1orjones tay1orjones left a comment

Choose a reason for hiding this comment

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

These make sense to me - thanks for including so much detail in the original issue!

How fun that you got to open and close the 10,000th issue on the repository! 🏅 🎉

@kodiakhq kodiakhq bot merged commit a8a128c into carbon-design-system:main Nov 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Several problems with selectorsFloatingMenus prop on ComposedModal
3 participants