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(modal): calciteModalClose should emit on close button click #7680
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome!
/** | ||
* When true, the component is open. | ||
*/ | ||
opened?: boolean; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Elijbet Let's look into refactoring this to follow <propName>Prop
, similar to other props, so components can specify their source-of-truth open prop. #proppropprop
const modalBeforeClose = page.waitForEvent("calciteModalBeforeClose"); | ||
const modalClose = page.waitForEvent("calciteModalClose"); | ||
|
||
const closeButton = await page.find("calcite-modal >>> .close"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this use the CSS
lookup object?
This seems low-risk to me. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💥💥💥
@@ -543,7 +568,7 @@ describe("calcite-modal accessibility checks", () => { | |||
modal.setProperty("open", true); | |||
await page.waitForChanges(); | |||
expect(await modal.isVisible()).toBe(true); | |||
const closeButton = await page.find("calcite-modal >>> .close"); | |||
const closeButton = await page.find(`calcite-modal >>> .${CSS.close}`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✨🧹✨
* origin/main: fix(list): remove unnecessary z-index (#7678) fix(sheet): calciteSheetClose should emit on scrim click (#7685) fix(modal): calciteModalClose should emit on close button click (#7680) fix(list-item): Do not call preventDefault on enter key within slotted actions (#7684) chore(locale): add extra info to default numbering system export (#7683) fix(input-date-picker): apply default numbering system to avoid browser inferring from locale (#7682) chore: release next fix(time-picker): focus corresponding input when nudge buttons are clicked (#7650) build(deps): update dependency @types/sortablejs to v1.15.2 (#7674) feat(flow): split up custom flow item interfaces (#7666)
🤖 I have created a release *beep* *boop* --- <details><summary>@esri/calcite-components: 1.8.0</summary> ## [1.8.0](https://github.com/Esri/calcite-design-system/compare/@esri/calcite-components@1.7.0...@esri/calcite-components@1.8.0) (2023-09-06) ### Features * **flow:** Split up custom flow item interfaces ([#7666](#7666)) ([6c22e7c](6c22e7c)) ### Bug Fixes * **action-group:** Honor flex-grow on slotted actions ([#7690](#7690)) ([0777234](0777234)) * **block:** Provide textual name on collapse and expansion to AT ([#7652](#7652)) ([85bd71b](85bd71b)) * **input-date-picker:** Apply default numbering system to avoid browser inferring from locale ([#7682](#7682)) ([3e1ed2d](3e1ed2d)) * **input-time-zone:** Prevent items from being deselected ([#7661](#7661)) ([c2dd436](c2dd436)) * **list-item:** Do not call preventDefault on enter key within slotted actions ([#7684](#7684)) ([c3261f0](c3261f0)) * **list:** Remove unnecessary z-index ([#7678](#7678)) ([1f4cd97](1f4cd97)) * **modal:** CalciteModalClose should emit on close button click ([#7680](#7680)) ([796bf90](796bf90)) * **sheet:** CalciteSheetClose should emit on scrim click ([#7685](#7685)) ([6ae963e](6ae963e)) * **sheet:** Update shadow style in display-mode="float" ([#7664](#7664)) ([851bedb](851bedb)) * **table:** Allow wrapping text in table-header and table-cell ([#7665](#7665)) ([15cb2bf](15cb2bf)) * **time-picker:** Focus corresponding input when nudge buttons are clicked ([#7650](#7650)) ([9c7d846](9c7d846)) </details> <details><summary>@esri/calcite-components-react: 1.8.0</summary> ## [1.8.0](https://github.com/Esri/calcite-design-system/compare/@esri/calcite-components-react@1.7.0...@esri/calcite-components-react@1.8.0) (2023-09-06) ### Bug Fixes * Resolve vite errors due to using expressions in dynamic imports ([#7671](#7671)) ([b4c1038](b4c1038)) ### Dependencies * The following workspace dependencies were updated * dependencies * @esri/calcite-components bumped from ^1.8.0-next.2 to ^1.8.0 </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Related Issue: #7655
Summary
opened
property for components that have it