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.

Sign up for GitHub

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

Jump to bottom

Calcite-Modal "Close" Button not Triggering calciteModalClose event #7655

Closed
2 of 3 tasks
tom12993 opened this issue Sep 1, 2023 · 6 comments
Closed
2 of 3 tasks

Calcite-Modal "Close" Button not Triggering calciteModalClose event #7655

tom12993 opened this issue Sep 1, 2023 · 6 comments
Assignees
@geospatialem
Labels
4 - verified Issues that have been released and confirmed resolved. ArcGIS Monitor Issues logged by ArcGIS Monitor team members. bug Bug reports for broken functionality. Issues should include a reproduction of the bug. calcite-components Issues specific to the @esri/calcite-components package. impact - p0 - emergency User set priority impact status of p0 - emergency p - high Issue should be addressed in the current milestone, impacts component or core functionality regression Issues that are caused by changes in a release, but were working before that.
Milestone
1.8.0 Maintenance...

Comments

@tom12993
Copy link

tom12993 commented Sep 1, 2023

Check existing issues

Actual Behavior

In calcite-modal when using the built-in "Close" button in the header, the calciteModalClose and calciteModalBeforeClose events aren't firing.

Expected Behavior

Using the "Close" button should trigger the calciteModalClose event.

Reproduction Sample

https://codepen.io/tom12993/pen/MWZePrK?editors=1001

Reproduction Steps

  1. Go to provided codepen link
  2. Observe that the Calcite version is set to 1.7.0
  3. Use the "Open modal" button to open the calcite-modal
  4. Use the "Cancel" button to close the calcite-modal
    a. Observe the Console, and see that the event is logged
  5. Use the "Open modal" button again to open the calcite-modal
  6. Use the "X" button in the header to close the calcite-modal
    a. Observe the Console, and see that the event is NOT logged
  7. Repeat in 1.6.1 and notice that the event is triggered both times.

Reproduction Version

1.7.0

Relevant Info

No response

Regression?

1.6.1

Priority impact

p0 - emergency

Impact

Modals aren't working as expected when closed via the "X" button. Prevents the modal from being opened a 2nd time in our case, because the modal's "open" state is never set to false due to the event not triggering. Also affects our behaviors that are expected to happen when our modals close.

Calcite package

  • @esri/calcite-components
  • @esri/calcite-components-react

Esri team

ArcGIS Monitor
@tom12993 tom12993 added 0 - new New issues that need assignment. bug Bug reports for broken functionality. Issues should include a reproduction of the bug. needs triage Planning workflow - pending design/dev review. labels Sep 1, 2023
@github-actions github-actions bot added ArcGIS Monitor Issues logged by ArcGIS Monitor team members. calcite-components Issues specific to the @esri/calcite-components package. impact - p0 - emergency User set priority impact status of p0 - emergency labels Sep 1, 2023
@geospatialem geospatialem added regression Issues that are caused by changes in a release, but were working before that. p - high Issue should be addressed in the current milestone, impacts component or core functionality labels Sep 1, 2023
@geospatialem geospatialem added this to the 1.7.1 Maintenance Release milestone Sep 1, 2023
@geospatialem geospatialem removed the needs triage Planning workflow - pending design/dev review. label Sep 1, 2023
@jcfranco
Copy link
Member

jcfranco commented Sep 1, 2023

@driskull Do you think you could look at this one? Possibly caused by #7470.

@geospatialem
Copy link
Member

This also seems to be occurring with sheet's calciteSheetClose event: https://codepen.io/geospatialem/pen/jOXrXQQ

@driskull
Copy link
Member

driskull commented Sep 5, 2023

@jcfranco I think the openCloseComponent will need to be updated to account for opened being the source of truth for some components. Do you want me to take that or @Elijbet?

@jcfranco
Copy link
Member

jcfranco commented Sep 5, 2023

Excellent sleuthing! 🕵️

Assigning to you for the moment since you're already taking a look. @Elijbet or myself can help out if needed.

@jcfranco jcfranco added 1 - assigned Issues that are assigned to a sprint and a team member. and removed 0 - new New issues that need assignment. labels Sep 5, 2023
driskull added a commit that referenced this issue Sep 5, 2023
@driskull
fix(modal): calciteModalClose should emit on close button click. #7655
5e8c64f
@driskull driskull mentioned this issue Sep 5, 2023
Merged
@geospatialem geospatialem mentioned this issue Sep 5, 2023
Closed
3 tasks
driskull added a commit that referenced this issue Sep 6, 2023
@driskull
fix(modal): calciteModalClose should emit on close button click (#7680)
796bf90 
**Related Issue:** #7655

## Summary

- Updates openCloseComponent helper to consider `opened` property for
components that have it
- Update modal to emit correctly on close click
- Add test
- @jcfranco for risk consideration.
@driskull driskull added 3 - installed Issues that have been merged to master branch and are ready for final confirmation. and removed 1 - assigned Issues that are assigned to a sprint and a team member. labels Sep 6, 2023
@github-actions github-actions bot assigned geospatialem and unassigned driskull Sep 6, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Sep 6, 2023

Installed and assigned for verification.

@geospatialem geospatialem added 4 - verified Issues that have been released and confirmed resolved. and removed 3 - installed Issues that have been merged to master branch and are ready for final confirmation. labels Sep 6, 2023
@geospatialem
Copy link
Member

Verified on main when the modal is closed via the scrim click, Esc key, and component's close button.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@geospatialem geospatialem

Labels
4 - verified Issues that have been released and confirmed resolved. ArcGIS Monitor Issues logged by ArcGIS Monitor team members. bug Bug reports for broken functionality. Issues should include a reproduction of the bug. calcite-components Issues specific to the @esri/calcite-components package. impact - p0 - emergency User set priority impact status of p0 - emergency p - high Issue should be addressed in the current milestone, impacts component or core functionality regression Issues that are caused by changes in a release, but were working before that.
Projects
None yet
Milestone
1.8.0 Maintenance Release
Development

No branches or pull requests
4 participants
@jcfranco @driskull @geospatialem @tom12993