-
Notifications
You must be signed in to change notification settings - Fork 77
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
feat(sheet): Add Sheet component #7561
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.
Great work 👏 @driskull !
🏗️ 🏗️ 🏗️ 👷♂️
Looks good, other than nitpick comments and replacing css with tailwind util classes.
Needs a11y testing since we are using scrim and suggest getting reviewed from more eyes before we install. We can move forward with installing this in upcoming release since it is a new component and has low risk.
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! ✨📄✨
transition: visibility 0ms linear var(--calcite-internal-animation-timing-medium), | ||
opacity var(--calcite-internal-animation-timing-medium) $easing-function; | ||
// the sheet should always use a dark scrim, regardless of light / dark mode - matches value in global.scss | ||
--calcite-sheet-scrim-background-internal: #{rgba($blk-240, 0.85)}; |
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.
Could we move these to the .container
level, to prevent overriding at the host level? We should also rename these to be --calcite-internal-<var-name>
to align with other private CSS props and event. cc @macandcheese
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.
Yeah I was wondering if we should do this as well. I copied this from Modal so maybe an issue for modal is needed as well? @jcfranco can you create a refactor issue for modal? Modal could use almost all of the same refactors from this review.
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.
@macandcheese I removed these since they are already on the container. Seems to be fine but could you verify?
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.
expect(documentClass).toEqual(false); | ||
}); | ||
|
||
describe("opening and closing behavior", () => { |
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 One more case for the open/close test util to cover. ✨🧪✨
@Prop() slottedInShell: boolean; | ||
|
||
/** | ||
* When `position` is `"inline-start"` or `"inline-end"`, specifies the width of the component. |
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.
Doc question, does this affect height when the writing-mode
is changed? We might need to fine-tune both modal
and sheet
's doc separately depending on this.
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.
@macandcheese do you know the answer to this one?
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.
AFAIK it would still just set the container width to the supplied default or overwritten css vars - maybe we could test for this? Any changes or doc could also apply to Shell Panel where we have similar width / height props / css vars.
`; | ||
simpleDarkMode.parameters = { modes: modesDarkDefault }; | ||
|
||
export const inlineStartfloat_TestOnly = (): string => |
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.
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.
@macandcheese could it be a browser version difference?
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.
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.
Ill put back some host styles and see if that fixes the screenshots.
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.
@macandcheese I think the screens are good now. Let me know if its good to merge
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 you kick off another Chromatic? Inset looks good locally but wasn't showing up before outside of Chromatic. I do still see the non-rounded border in Safari. Modal / Shell Panel in 'float' don't show this so maybe we just missed one of the styles?
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.
@macandcheese when we set the border radius, we need to set overflow:hidden so the panel doesn't go outside of where it should.
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! Approved on Chromatic - Safari border radius issue in float
is resolved looking locally.
**Related Issue:** #6391 ## Summary - Adds new `calcite-sheet` component. - Adds e2e tests - Adds screenshot tests - Adds html page with examples - Modeled after the Modal component. - Added to index page - Added to stencil config ## After merged - [Shell] Add "Sheet" Slot - Will try to do for this release if it gets merged in time - Issue: #7154 - PR: #7579 ## Future - Resizable (need issue) @macandcheese ## API ### Example ```html <calcite-sheet open label="libero nunc" position="inline-start"> <calcite-panel closable heading="Ultrices neque" ><p> Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum. </p> <calcite-button slot="footer" width="half" appearance="outline">tincidunt lobortis</calcite-button> <calcite-button slot="footer" width="half" appearance="outline">amet porttitor</calcite-button> </calcite-panel> <script> document.addEventListener("calcitePanelClose", () => { document.querySelector("calcite-sheet").open = false; }); </script> </calcite-sheet> ``` ## Properties | Property | Attribute | Description | Type | Default | | ---------------------- | ------------------------ | --------------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------- | ---------------- | | `beforeClose` | -- | Passes a function to run before the component closes. | `(el: HTMLCalciteSheetElement) => Promise<void>` | `undefined` | | `displayMode` | `display-mode` | Specifies the display mode - `"float"` (content is separated detached), or `"overlay"` (displays on top of center content). | `"float" \| "overlay"` | `"overlay"` | | `escapeDisabled` | `escape-disabled` | When `true`, disables the default close on escape behavior. | `boolean` | `false` | | `focusTrapDisabled` | `focus-trap-disabled` | When `true`, prevents focus trapping. | `boolean` | `false` | | `heightScale` | `height-scale` | When `position` is `"block-start"` or `"block-end"`, specifies the height of the component. | `"l" \| "m" \| "s"` | `"m"` | | `label` _(required)_ | `label` | Specifies the label of the component. | `string` | `undefined` | | `open` | `open` | When `true`, displays and positions the component. | `boolean` | `false` | | `outsideCloseDisabled` | `outside-close-disabled` | When `true`, disables the closing of the component when clicked outside. | `boolean` | `false` | | `position` | `position` | When `true`, disables the closing of the component when clicked outside. | `"block-end" \| "block-start" \| "inline-end" \| "inline-start"` | `"inline-start"` | | `widthScale` | `width-scale` | When `position` is `"inline-start"` or `"inline-end"`, specifies the width of the component. | `"l" \| "m" \| "s"` | `"m"` | ## Events | Event | Description | Type | | ------------------------- | -------------------------------------------------------------------------------------------------------- | ------------------- | | `calciteSheetBeforeClose` | Fires when the component is requested to be closed and before the closing transition begins. | `CustomEvent<void>` | | `calciteSheetBeforeOpen` | Fires when the component is added to the DOM but not rendered, and before the opening transition begins. | `CustomEvent<void>` | | `calciteSheetClose` | Fires when the component is closed and animation is complete. | `CustomEvent<void>` | | `calciteSheetOpen` | Fires when the component is open and animation is complete. | `CustomEvent<void>` | ## Methods ### `setFocus() => Promise<void>` Sets focus on the component's "close" button (the first focusable item). #### Returns Type: `Promise<void>` ### `updateFocusTrapElements() => Promise<void>` Updates the element(s) that are used within the focus-trap of the component. #### Returns Type: `Promise<void>` ## CSS Custom Properties | Name | Description | | ---------------------------------- | ---------------------------------------------------------------------------------------------------- | | `--calcite-sheet-height` | When `position` is `"block-start"` or `"block-end"`, specifies the height of the component. | | `--calcite-sheet-max-height` | When `position` is `"block-start"` or `"block-end"`, specifies the maximum height of the component. | | `--calcite-sheet-max-width` | When `position` is `"inline-start"` or `"inline-end"`, specifies the maximum width of the component. | | `--calcite-sheet-min-height` | When `position` is `"block-start"` or `"block-end"`, specifies the minimum height of the component. | | `--calcite-sheet-min-width` | When `position` is `"inline-start"` or `"inline-end"`, specifies the minimum width of the component. | | `--calcite-sheet-scrim-background` | Specifies the background color of the sheet scrim. | | `--calcite-sheet-width` | When `position` is `"inline-start"` or `"inline-end"`, specifies the width of the component. | --------- Co-authored-by: Adam <adam@tirel.la>
…input * origin/main: chore: release next feat(sheet): Add Sheet component (#7561)
**Related Issue:** #7154 ## Summary - depends on #7561 - Adds `sheets` slot to the shell where users can place `calcite-sheet` components - Adds screenshot test - Adds example HTML cc @macandcheese --------- Co-authored-by: Adam <adam@tirel.la>
🤖 I have created a release *beep* *boop* --- <details><summary>@esri/calcite-components: 1.7.0</summary> ## [1.7.0](https://github.com/Esri/calcite-design-system/compare/@esri/calcite-components@1.6.1...@esri/calcite-components@1.7.0) (2023-09-01) ### Features * **action-bar, action-pad, action-group:** Add label properties for group context ([#7415](#7415)) ([b34f36d](b34f36d)) * **combobox:** Add single-persist selection mode ([#7583](#7583)) ([dab06a3](dab06a3)) * **flow:** Add support for custom flow-item elements ([#7608](#7608)) ([197adfe](197adfe)) * **input-date-picker:** Normalize year to current century for user typed values only ([#7638](#7638)) ([a1db718](a1db718)) * **input-number:** Add integer property ([#7646](#7646)) ([cd66a6d](cd66a6d)) * **input-time-picker:** Support fractional seconds ([#7532](#7532)) ([c2bf34b](c2bf34b)) * **meter:** Add Meter component ([#7401](#7401)) ([47163ed](47163ed)) * **sheet:** Add Sheet component ([#7561](#7561)) ([f12a393](f12a393)) * **sheet:** Update default widths ([#7617](#7617)) ([47d2c0b](47d2c0b)) * **shell:** Add "Sheets" Slot ([#7579](#7579)) ([e798765](e798765)) * **table:** Add Table and related components ([#7607](#7607)) ([b067e72](b067e72)) ### Bug Fixes * **accordion, accordion-item:** Improve a11y ([#7560](#7560)) ([b5170b6](b5170b6)) * Add drag styles for improved UX ([#7644](#7644)) ([afbb764](afbb764)) * **block, block-section:** Improve a11y ([#7557](#7557)) ([1f44f6b](1f44f6b)) * **chip-group:** Add existence checks ([#7586](#7586)) ([5ca64f1](5ca64f1)) * **color-picker:** Update value when alphaChannel is toggled ([#7563](#7563)) ([1f753dd](1f753dd)) * **combobox:** Prevent deselecting items via keyboard in single-persist mode ([#7634](#7634)) ([4f5f8b0](4f5f8b0)) * **combobox:** Update combobox height to follow design spec ([#7558](#7558)) ([ec08845](ec08845)) * **date-picker:** Set start of week to monday in zh-CN ([#7578](#7578)) ([7e385cb](7e385cb)) * **dropdown:** Prevents navigating dropdown items with Tab key ([#7527](#7527)) ([3ea658d](3ea658d)) * Ensure label only focuses the first labelable child ([#7553](#7553)) ([426159c](426159c)) * **flow:** Catch error when beforeBack promise is rejected ([#7601](#7601)) ([cb70671](cb70671)) * **input-date-picker, input-time-picker:** Do not show dropdown affordance when read-only ([#7559](#7559)) ([5a3f19c](5a3f19c)) * **input, input-number:** Correctly sanitize numbers when pasting string with 'e' ([#7648](#7648)) ([b8bc11c](b8bc11c)) * **list, sortable-list, value-list:** Emit calciteListOrderChange when dragging between lists ([#7614](#7614)) ([4653581](4653581)) * **list:** Fixes dragging nested list items ([#7555](#7555)) ([c25f7b3](c25f7b3)) * **list:** Stop emitting calciteListChange when a list-item is disabled or closed. ([#7624](#7624)) ([7008463](7008463)) * **loader:** Tweak loading animations to work in Safari ([#7564](#7564)) ([2103654](2103654)) * **modal:** Catch error when beforeClose promise is rejected ([#7600](#7600)) ([70405d0](70405d0)) * **modal:** Handle removal of open attribute and prevent multiple beforeClose calls ([#7470](#7470)) ([f31588f](f31588f)) * **rating:** Adds focus outline on click ([#7341](#7341)) ([af30073](af30073)) * **segmented-control:** Refresh items when added dynamically ([#7567](#7567)) ([2e36eb3](2e36eb3)) * **split-button:** Update divider and borders to follow design spec ([#7568](#7568)) ([8df59ab](8df59ab)) * **tree-item:** Move focus outline to item label area ([#7581](#7581)) ([1327cef](1327cef)) * **tree-item:** Updates state when selection changes programmatically for `selection-mode='ancestors'` ([#7572](#7572)) ([40758c5](40758c5)) * **tree:** Improve keyboard navigation ([#7618](#7618)) ([826a5cb](826a5cb)) </details> <details><summary>@esri/calcite-components-react: 1.7.0</summary> ## [1.7.0](https://github.com/Esri/calcite-design-system/compare/@esri/calcite-components-react@1.6.1...@esri/calcite-components-react@1.7.0) (2023-09-01) ### Bug Fixes * Make sure components are defined in environments like in codesandbox ([#7632](#7632)) ([7005cce](7005cce)) ### Dependencies * The following workspace dependencies were updated * dependencies * @esri/calcite-components bumped from ^1.7.0-next.22 to ^1.7.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: #6391
Summary
calcite-sheet
component.After merged
Future
API
Example
Properties
beforeClose
(el: HTMLCalciteSheetElement) => Promise<void>
undefined
displayMode
display-mode
"float"
(content is separated detached), or"overlay"
(displays on top of center content)."float" | "overlay"
"overlay"
escapeDisabled
escape-disabled
true
, disables the default close on escape behavior.boolean
false
focusTrapDisabled
focus-trap-disabled
true
, prevents focus trapping.boolean
false
heightScale
height-scale
position
is"block-start"
or"block-end"
, specifies the height of the component."l" | "m" | "s"
"m"
label
(required)label
string
undefined
open
open
true
, displays and positions the component.boolean
false
outsideCloseDisabled
outside-close-disabled
true
, disables the closing of the component when clicked outside.boolean
false
position
position
true
, disables the closing of the component when clicked outside."block-end" | "block-start" | "inline-end" | "inline-start"
"inline-start"
widthScale
width-scale
position
is"inline-start"
or"inline-end"
, specifies the width of the component."l" | "m" | "s"
"m"
Events
calciteSheetBeforeClose
CustomEvent<void>
calciteSheetBeforeOpen
CustomEvent<void>
calciteSheetClose
CustomEvent<void>
calciteSheetOpen
CustomEvent<void>
Methods
setFocus() => Promise<void>
Sets focus on the component's "close" button (the first focusable item).
Returns
Type:
Promise<void>
updateFocusTrapElements() => Promise<void>
Updates the element(s) that are used within the focus-trap of the component.
Returns
Type:
Promise<void>
CSS Custom Properties
--calcite-sheet-height
position
is"block-start"
or"block-end"
, specifies the height of the component.--calcite-sheet-max-height
position
is"block-start"
or"block-end"
, specifies the maximum height of the component.--calcite-sheet-max-width
position
is"inline-start"
or"inline-end"
, specifies the maximum width of the component.--calcite-sheet-min-height
position
is"block-start"
or"block-end"
, specifies the minimum height of the component.--calcite-sheet-min-width
position
is"inline-start"
or"inline-end"
, specifies the minimum width of the component.--calcite-sheet-scrim-background
--calcite-sheet-width
position
is"inline-start"
or"inline-end"
, specifies the width of the component.