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(16337): close DatePicker calendar on away click when inside Modal or ComposedModal #16758

Closed

Conversation

2nikhiltom
Copy link
Contributor

@2nikhiltom 2nikhiltom commented Jun 12, 2024

Closes #16337

The root bug with DatePicker was : When using the datepicker with a range selection, the end date calendar does not close when the user clicks away, causing it to always stay open (Issue -> PR)

Fix for #16337 is dependent on PR
Also this issue was happing because evt.stopPropagation() was added on Mousedown event to add support for Nest modal

To fix #16337 : I thought to keep this action behind a prop

  • Introduced a new prop isNested, in modal.tsx.
  • The event handler will now run evt.stopPropagation() only when the isNested prop is present.
image
  • Additionally, we can update the documentation to include guidelines advising against the use of DatePickers within nested modals.

I am looking for reviews and suggestions on this.
If isNested prop works as a fix , I will be implementing this is in ComposedModal as well

Testing / Reviewing

  • Take pull in local
  • Open Modal storybook->default
  • Select start and end date in the datepicker
  • Open end date calendar, do not select any date and click outside of calendar and verify if calendar closes

Verify the functionlity when isNested is passed

  • Pass isNested as a prop in Modal
  • Select start and end date in the datepicker
image
  • Open end date calendar, do not select any date and click outside of calendar and verify that does not close

@2nikhiltom 2nikhiltom requested review from a team as code owners June 12, 2024 14:19
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The changes in fixEventsPlugin.js will be made available from #16742

Copy link

netlify bot commented Jun 12, 2024

Deploy Preview for v11-carbon-react ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 26b1a81
🔍 Latest deploy log https://app.netlify.com/sites/v11-carbon-react/deploys/6669b27d16018e00095205aa
😎 Deploy Preview https://deploy-preview-16758--v11-carbon-react.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Jun 12, 2024

Deploy Preview for carbon-elements ready!

Name Link
🔨 Latest commit 26b1a81
🔍 Latest deploy log https://app.netlify.com/sites/carbon-elements/deploys/6669b27d8b7b98000879e1a5
😎 Deploy Preview https://deploy-preview-16758--carbon-elements.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@@ -45,6 +47,7 @@ export const Default = () => {
modalHeading="Add a custom domain"
modalLabel="Account resources"
primaryButtonText="Add"
//isNested
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this comment can be removed!!

@@ -109,9 +109,10 @@ export default (config) => (fp) => {
if (inputTo.value) {
fp.setDate(
[inputFrom.value, inputTo.value],
true,
false,
Copy link
Contributor

Choose a reason for hiding this comment

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

Does previously sending the second value as true , was forcefully firing onChange event ?

Copy link
Contributor

@riddhybansal riddhybansal left a comment

Choose a reason for hiding this comment

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

This looks like a great hack @2nikhiltom . Great work I must say !!

@2nikhiltom
Copy link
Contributor Author

fixed via #17072

@2nikhiltom 2nikhiltom closed this Sep 4, 2024
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]: <DatePicker> hosted inside of <Modal> or <ComposedModal> does not close calendar when clicking outside
2 participants