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

Sheet: Close event not triggered with calciteSheetClose event #7681

Closed
2 of 3 tasks
geospatialem opened this issue Sep 5, 2023 · 7 comments
Closed
2 of 3 tasks

Sheet: Close event not triggered with calciteSheetClose event #7681

geospatialem opened this issue Sep 5, 2023 · 7 comments
Assignees
@geospatialem
Labels
4 - verified Issues that have been released and confirmed resolved. bug Bug reports for broken functionality. Issues should include a reproduction of the bug. Calcite (dev) Issues logged by Calcite developers. calcite-components Issues specific to the @esri/calcite-components package. estimate - 2 Small fix or update, may require updates to tests. impact - p1 - need for current milestone User set priority impact status of p1 - need for current milestone p - high Issue should be addressed in the current milestone, impacts component or core functionality
Milestone
1.8.0 Maintenance...

Comments

@geospatialem
Copy link
Member

Check existing issues

Actual Behavior

Sheet's close event is not being triggered

Expected Behavior

Sheets close event is triggered with calciteSheetClose.

Reproduction Sample

https://codepen.io/geospatialem/pen/jOXrXQQ

Reproduction Steps

  1. Open the sample
  2. Try to close the sheet by selecting the scrim
  3. Re-load the sample
  4. Try to close the sheet by selecting the close button
  5. Re-load the sample
  6. Lastly, try to close the sheet by pressing the Esc key

Reproduction Version

1.7.0

Relevant Info

No response

Regression?

No response

Priority impact

p1 - need for current milestone

Impact

No response

Calcite package

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

Esri team

Calcite (dev)
@geospatialem geospatialem added bug Bug reports for broken functionality. Issues should include a reproduction of the bug. p - high Issue should be addressed in the current milestone, impacts component or core functionality 2 - in development Issues that are actively being worked on. estimate - 2 Small fix or update, may require updates to tests. labels Sep 5, 2023
@geospatialem geospatialem added this to the 1.8.0 Maintenance Release milestone Sep 5, 2023
@github-actions github-actions bot added impact - p1 - need for current milestone User set priority impact status of p1 - need for current milestone Calcite (dev) Issues logged by Calcite developers. calcite-components Issues specific to the @esri/calcite-components package. labels Sep 5, 2023
@geospatialem
Copy link
Member Author

Related to #7655, pertaining to the sheet component.

driskull added a commit that referenced this issue Sep 6, 2023
@driskull
fix(sheet): calciteSheetClose should emit on scrim click. #7681
d7b45d8
@driskull driskull mentioned this issue Sep 6, 2023
Merged
driskull added a commit that referenced this issue Sep 6, 2023
@driskull
fix(sheet): calciteSheetClose should emit on scrim click (#7685)
6ae963e 
**Related Issue:** #7681

## Summary

- Depends on #7680
- Update sheet to emit correctly on scrim 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 2 - in development Issues that are actively being worked on. labels Sep 6, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Sep 6, 2023

Installed and assigned for verification.

@geospatialem
Copy link
Member Author

Looks good when clicking the scrim on the main branch. 👍🏻

@driskull Would we expect the calciteSheetClose to fire when the Escape key is also pressed?

@driskull
Copy link
Member

driskull commented Sep 6, 2023

Yes, is it not firing?

@geospatialem
Copy link
Member Author

It doesn't seem to on main with code taken from https://codepen.io/geospatialem/pen/jOXrXQQ (there are some unstable tests impacting the next releases currently).

@driskull
Copy link
Member

driskull commented Sep 6, 2023

Seems to work fine locally.

@geospatialem
Copy link
Member Author

Ah, thanks! Yes, we're good, was a testing issue when the component was opened and not using our Core concepts.

Closing, as we're verified on the main branch with the scrim click and Escape key. 🎉

@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
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. bug Bug reports for broken functionality. Issues should include a reproduction of the bug. Calcite (dev) Issues logged by Calcite developers. calcite-components Issues specific to the @esri/calcite-components package. estimate - 2 Small fix or update, may require updates to tests. impact - p1 - need for current milestone User set priority impact status of p1 - need for current milestone p - high Issue should be addressed in the current milestone, impacts component or core functionality
Projects
None yet
Milestone
1.8.0 Maintenance Release
Development

No branches or pull requests
2 participants
@driskull @geospatialem