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

feat(popover): migrate s2 popover #3365

Merged
merged 1 commit into from
Jan 31, 2025
Merged

feat(popover): migrate s2 popover #3365

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/itchy-shrimps-help.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@spectrum-css/picker": patch
---

S2 Popover has an updated default position, bottom-start. The correct `.spectrum-Popover--bottom-start` class was added to the open popover selector blocks for picker so that the spacing from the picker to the popover is correctly adjusted.
17 changes: 17 additions & 0 deletions .changeset/rotten-meals-share.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
---
"@spectrum-css/popover": major
---

S2 Popover Migration

The S2 popover using new tokens for border color, padding, corner radius and elevation/drop shadows.

Renamed Mods

| Old mod | New mod |
| --------------------------------------------- | ------------------------------------ |
| `--mod-popover-shadow-blur` | `--mod-popover-drop-shadow-blur` |
| `--mod-popover-shadow-color` | `--mod-popover-drop-shadow-color` |
| `--mod-popover-shadow-horizontal` | `--mod-popover-drop-shadow-x` |
| `--mod-popover-shadow-vertical` | `--mod-popover-drop-shadow-y` |
| `--mod-popover-content-area-spacing-vertical` | `--mod-popover-content-area-spacing` |
8 changes: 8 additions & 0 deletions .changeset/twenty-ravens-divide.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
"@spectrum-css/coachmark": minor
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Just making sure here: the changes in the coachmark CSS are actually a result of the breaking changes from popover. Would that still be considered a minor change in the coachmark CSS?

---

CSS references to popover custom properties/tokens were updated to reflect some mod name changes in the S2 popover (#3365). The corner radius token was updated to correspond to the new S2 popover corner radius, as well as some updated popover mod names:

- `--mod-popover-content-area-spacing-vertical` was updated to `--mod-popover-content-area-spacing`.
- `--spectrum-border-width-100` was updated to `--spectrum-corner-radius-large-default`
4 changes: 2 additions & 2 deletions components/coachmark/dist/metadata.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@
"--spectrum-body-sans-serif-font-style",
"--spectrum-body-sans-serif-font-weight",
"--spectrum-border-width-100",
"--spectrum-corner-radius-100",
"--spectrum-corner-radius-large-default",
"--spectrum-heading-color",
"--spectrum-heading-line-height",
"--spectrum-heading-sans-serif-font-weight",
Expand All @@ -86,7 +86,7 @@
"--mod-buttongroup-justify-content",
"--mod-buttongroup-spacing",
"--mod-popover-border-width",
"--mod-popover-content-area-spacing-vertical",
"--mod-popover-content-area-spacing",
"--mod-popover-corner-radius"
],
"high-contrast": []
Expand Down
4 changes: 2 additions & 2 deletions components/coachmark/index.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@
/* @passthrough start */
--mod-buttongroup-justify-content: flex-end;
--mod-popover-border-width: var(--spectrum-border-width-100);
--mod-popover-corner-radius: var(--spectrum-corner-radius-100);
--mod-popover-content-area-spacing-vertical: 0;
--mod-popover-corner-radius: var(--spectrum-corner-radius-large-default);
--mod-popover-content-area-spacing: 0;
Copy link
Collaborator Author

@marissahuysentruyt marissahuysentruyt Jan 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I updated this mod name because I changed popover-content-area-spacing in the popover CSS- there's now padding on all sides of popover instead of just on the top & bottom. However, I am unsure if this is actually the correct styling for the popovers in coachmark. Is this something to address in the coachmark PR #3412? Regardless, we do have this captured in CSS-1067

With the new padding in Popover (especially noticeable in the action menu popover), which means removing --mod-popover-content-area-spacing completely.
Screenshot 2025-01-13 at 10 52 04 AM

Keeping --mod-popover-content-area-spacing: 0, which results in no padding
Screenshot 2025-01-13 at 10 51 55 AM

We might need something like this:
padding: var(--mod-popover-content-area-spacing, var(--spectrum-popover-content-area-spacing)) 0; to mimic the old popover vertical padding. What do y'all think?

--mod-button-edge-to-visual-only: 9px;
/* @passthrough end */

Expand Down
8 changes: 4 additions & 4 deletions components/picker/dist/metadata.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"sourceFile": "index.css",
"selectors": [
".spectrum-Picker",
".spectrum-Picker + .spectrum-Popover--bottom.is-open",
".spectrum-Picker + .spectrum-Popover--bottom-start.is-open",
".spectrum-Picker .spectrum-Picker-icon",
".spectrum-Picker .spectrum-Picker-label",
".spectrum-Picker .spectrum-ProgressCircle",
Expand All @@ -13,11 +13,11 @@
".spectrum-Picker--quiet:focus-visible:after",
".spectrum-Picker--sideLabel",
".spectrum-Picker--sizeL",
".spectrum-Picker--sizeL + .spectrum-Popover--bottom.is-open",
".spectrum-Picker--sizeL + .spectrum-Popover--bottom-start.is-open",
".spectrum-Picker--sizeS",
".spectrum-Picker--sizeS + .spectrum-Popover--bottom.is-open",
".spectrum-Picker--sizeS + .spectrum-Popover--bottom-start.is-open",
".spectrum-Picker--sizeXL",
".spectrum-Picker--sizeXL + .spectrum-Popover--bottom.is-open",
".spectrum-Picker--sizeXL + .spectrum-Popover--bottom-start.is-open",
".spectrum-Picker-label",
".spectrum-Picker-label.is-placeholder",
".spectrum-Picker-label.is-placeholder:active",
Expand Down
10 changes: 5 additions & 5 deletions components/picker/index.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@
--spectrum-picker-spacing-top-to-disclosure-icon: var(--spectrum-field-top-to-disclosure-icon-100);
--spectrum-picker-spacing-edge-to-disclosure-icon: var(--spectrum-field-end-edge-to-disclosure-icon-100);

& + .spectrum-Popover--bottom.is-open {
& + .spectrum-Popover--bottom-start.is-open {
--spectrum-picker-spacing-picker-to-popover: var(--spectrum-component-to-menu-medium);
}

Expand Down Expand Up @@ -124,7 +124,7 @@
--spectrum-picker-spacing-top-to-disclosure-icon: var(--spectrum-field-top-to-disclosure-icon-75);
--spectrum-picker-spacing-edge-to-disclosure-icon: var(--spectrum-field-end-edge-to-disclosure-icon-75);

& + .spectrum-Popover--bottom.is-open {
& + .spectrum-Popover--bottom-start.is-open {
--spectrum-picker-spacing-picker-to-popover: var(--spectrum-component-to-menu-small);
}
}
Expand All @@ -146,7 +146,7 @@
--spectrum-picker-spacing-top-to-disclosure-icon: var(--spectrum-field-top-to-disclosure-icon-200);
--spectrum-picker-spacing-edge-to-disclosure-icon: var(--spectrum-field-end-edge-to-disclosure-icon-200);

& + .spectrum-Popover--bottom.is-open {
& + .spectrum-Popover--bottom-start.is-open {
--spectrum-picker-spacing-picker-to-popover: var(--spectrum-component-to-menu-large);
}
}
Expand All @@ -168,7 +168,7 @@
--spectrum-picker-spacing-top-to-disclosure-icon: var(--spectrum-field-top-to-disclosure-icon-300);
--spectrum-picker-spacing-edge-to-disclosure-icon: var(--spectrum-field-end-edge-to-disclosure-icon-300);

& + .spectrum-Popover--bottom.is-open {
& + .spectrum-Popover--bottom-start.is-open {
--spectrum-picker-spacing-picker-to-popover: var(--spectrum-component-to-menu-extra-large);
}
}
Expand Down Expand Up @@ -409,7 +409,7 @@
margin-inline-end: var(--mod-picker-spacing-icon-to-disclosure-icon, var(--spectrum-picker-spacing-icon-to-disclosure-icon));
}

.spectrum-Picker + .spectrum-Popover--bottom.is-open {
.spectrum-Picker + .spectrum-Popover--bottom-start.is-open {
transform: translateY(var(--mod-picker-spacing-picker-to-popover, var(--spectrum-picker-spacing-picker-to-popover)));
}

Expand Down
5 changes: 4 additions & 1 deletion components/picker/stories/template.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,11 +143,14 @@ export const Template = ({
const popoverMarkup = popoverContent.length !== 0 ? Popover({
isOpen: isOpen && !isDisabled && !isLoading,
withTip: false,
position: "bottom",
position: "bottom-start",
Copy link
Collaborator Author

@marissahuysentruyt marissahuysentruyt Jan 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

bottom-start is from main & s2-foundations-redux. There's not explicit direction on the popover's positioning within picker that I found, but I don't think I see an issue with this change. Combobox, action menu, contextual help, date picker all use bottom-start.

The change here I thought then required the CSS selector changes, so if we're not cool with that right now, I'm happy to revert if there are any concerns.

isQuiet,
content: popoverContent,
size,
customStyles: customPopoverStyles,
popoverWrapperStyles: {
"display": "block",
},
}, context) : "";

const helpTextMarkup = helpText ? HelpText({
Expand Down
43 changes: 23 additions & 20 deletions components/popover/dist/metadata.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,48 +104,51 @@
"--mod-popover-background-color",
"--mod-popover-border-color",
"--mod-popover-border-width",
"--mod-popover-content-area-spacing-vertical",
"--mod-popover-content-area-spacing",
"--mod-popover-corner-radius",
"--mod-popover-drop-shadow-blur",
"--mod-popover-drop-shadow-color",
"--mod-popover-drop-shadow-x",
"--mod-popover-drop-shadow-y",
"--mod-popover-filter",
"--mod-popover-pointer-edge-spacing",
"--mod-popover-pointer-height",
"--mod-popover-pointer-width",
"--mod-popover-shadow-blur",
"--mod-popover-shadow-color",
"--mod-popover-shadow-horizontal",
"--mod-popover-shadow-vertical"
"--mod-popover-pointer-width"
],
"component": [
"--spectrum-popover-animation-distance",
"--spectrum-popover-background-color",
"--spectrum-popover-border-color",
"--spectrum-popover-border-color-rgb",
"--spectrum-popover-border-color-with-transparency",
"--spectrum-popover-border-opacity",
"--spectrum-popover-border-transparency",
"--spectrum-popover-border-width",
"--spectrum-popover-content-area-spacing-vertical",
"--spectrum-popover-content-area-spacing",
"--spectrum-popover-corner-radius",
"--spectrum-popover-drop-shadow-blur",
"--spectrum-popover-drop-shadow-color",
"--spectrum-popover-drop-shadow-x",
"--spectrum-popover-drop-shadow-y",
"--spectrum-popover-edge-to-content-area",
"--spectrum-popover-filter",
"--spectrum-popover-pointer-edge-offset",
"--spectrum-popover-pointer-edge-spacing",
"--spectrum-popover-pointer-height",
"--spectrum-popover-pointer-width",
"--spectrum-popover-shadow-blur",
"--spectrum-popover-shadow-color",
"--spectrum-popover-shadow-horizontal",
"--spectrum-popover-shadow-vertical",
"--spectrum-popover-tip-height",
"--spectrum-popover-tip-width",
"--spectrum-popover-top-to-content-area"
"--spectrum-popover-tip-width"
],
"global": [
"--spectrum-animation-duration-0",
"--spectrum-animation-duration-100",
"--spectrum-background-layer-2-color",
"--spectrum-border-width-100",
"--spectrum-corner-radius-100",
"--spectrum-drop-shadow-blur",
"--spectrum-drop-shadow-color",
"--spectrum-drop-shadow-x",
"--spectrum-drop-shadow-y",
"--spectrum-gray-400",
"--spectrum-corner-radius-large-default",
"--spectrum-drop-shadow-elevated-blur",
"--spectrum-drop-shadow-elevated-color",
"--spectrum-drop-shadow-elevated-x",
"--spectrum-drop-shadow-elevated-y",
"--spectrum-gray-200-rgb",
"--spectrum-spacing-100"
],
"passthroughs": [],
Expand Down
54 changes: 30 additions & 24 deletions components/popover/index.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,61 +13,60 @@
@import "@spectrum-css/commons/overlay.css";

.spectrum-Popover {
/* animation distance is equal to and responsible for popover offset */
/* animation distance is equal to and responsible for popover offset ("spacing" tip to source) */
--spectrum-popover-animation-distance: var(--spectrum-spacing-100);

--spectrum-popover-background-color: var(--spectrum-background-layer-2-color);
--spectrum-popover-border-color: var(--spectrum-gray-400);

/* TODO: Cannot use popover-border-color token until the value no longer has the rgba() function alpha/opacity. We should be able to refactor instances of `--spectrum-popover-border-color-rgb` with `--spectrum-popover-border-color` once this is available. */
/* --spectrum-popover-border-color-default: var(--spectrum-popover-border-color); */
--spectrum-popover-border-color-rgb: var(--spectrum-gray-200-rgb);
--spectrum-popover-border-transparency: var(--spectrum-popover-border-opacity);
--spectrum-popover-border-color-with-transparency: rgba(var(--spectrum-popover-border-color-rgb), var(--spectrum-popover-border-transparency));

--spectrum-popover-border-width: var(--spectrum-border-width-100);

/* popover inner padding */
--spectrum-popover-content-area-spacing-vertical: var(--spectrum-popover-top-to-content-area);
--spectrum-popover-content-area-spacing: var(--spectrum-popover-edge-to-content-area);

/* popover drop shadow */
--spectrum-popover-shadow-horizontal: var(--spectrum-drop-shadow-x);
--spectrum-popover-shadow-vertical: var(--spectrum-drop-shadow-y);
--spectrum-popover-shadow-blur: var(--spectrum-drop-shadow-blur);
--spectrum-popover-shadow-color: var(--spectrum-drop-shadow-color);
--spectrum-popover-drop-shadow-x: var(--spectrum-drop-shadow-elevated-x);
--spectrum-popover-drop-shadow-y: var(--spectrum-drop-shadow-elevated-y);
--spectrum-popover-drop-shadow-blur: var(--spectrum-drop-shadow-elevated-blur);
--spectrum-popover-drop-shadow-color: var(--spectrum-drop-shadow-elevated-color);

/* popover corner radius */
--spectrum-popover-corner-radius: var(--spectrum-corner-radius-100);
--spectrum-popover-corner-radius: var(--spectrum-corner-radius-large-default);

/* pointer tip dimensions */
--spectrum-popover-pointer-width: var(--spectrum-popover-tip-width);
--spectrum-popover-pointer-height: var(--spectrum-popover-tip-height);

/* pointer tip - default spacing to edge - corner radius plus half of tip width to neutralize override */
--spectrum-popover-pointer-edge-offset: calc(var(--spectrum-corner-radius-100) + (var(--spectrum-popover-tip-width) / 2));
--spectrum-popover-pointer-edge-offset: calc(var(--spectrum-popover-corner-radius) + (var(--spectrum-popover-tip-width) / 2));

/* pointer tip - spacing to edge will center pointer to source - apply in markup by setting '--spectrum-popover-pointer-edge-offset' value == half of source */
--spectrum-popover-pointer-edge-spacing: calc(var(--spectrum-popover-pointer-edge-offset) - (var(--spectrum-popover-tip-width) / 2));
}

/* windows high contrast mode */
@media (forced-colors: active) {
.spectrum-Popover {
--highcontrast-popover-border-color: CanvasText;
}
}

.spectrum-Popover {
--spectrum-popover-filter: drop-shadow(var(--mod-popover-shadow-horizontal, var(--spectrum-popover-shadow-horizontal)) var(--mod-popover-shadow-vertical, var(--spectrum-popover-shadow-vertical)) var(--mod-popover-shadow-blur, var(--spectrum-popover-shadow-blur)) var(--mod-popover-shadow-color, var(--spectrum-popover-shadow-color)));
--spectrum-popover-filter: drop-shadow(var(--mod-popover-drop-shadow-x, var(--spectrum-popover-drop-shadow-x)) var(--mod-popover-drop-shadow-y, var(--spectrum-popover-drop-shadow-y)) var(--mod-popover-drop-shadow-blur, var(--spectrum-popover-drop-shadow-blur)) var(--mod-popover-drop-shadow-color, var(--spectrum-popover-drop-shadow-color)));
@extend %spectrum-overlay;

box-sizing: border-box;
position: absolute;

outline: none; /* Hide focus outline */

padding: var(--mod-popover-content-area-spacing-vertical, var(--spectrum-popover-content-area-spacing-vertical)) 0;
padding: var(--mod-popover-content-area-spacing, var(--spectrum-popover-content-area-spacing));

display: inline-flex;
flex-direction: column;

border-style: solid;
border-radius: var(--mod-popover-corner-radius, var(--spectrum-popover-corner-radius));
border-color: var(--highcontrast-popover-border-color, var(--mod-popover-border-color, var(--spectrum-popover-border-color)));
border-color: var(--highcontrast-popover-border-color, var(--mod-popover-border-color, var(--spectrum-popover-border-color-with-transparency)));

border-width: var(--mod-popover-border-width, var(--spectrum-popover-border-width));

background-color: var(--mod-popover-background-color, var(--spectrum-popover-background-color));
Expand All @@ -81,7 +80,7 @@
stroke-linecap: square;
stroke-linejoin: miter;
fill: var(--mod-popover-background-color, var(--spectrum-popover-background-color));
stroke: var(--highcontrast-popover-border-color, var(--mod-popover-border-color, var(--spectrum-popover-border-color)));
stroke: var(--highcontrast-popover-border-color, var(--mod-popover-border-color, var(--spectrum-popover-border-color-with-transparency)));
stroke-width: var(--mod-popover-border-width, var(--spectrum-popover-border-width));
}
}
Expand Down Expand Up @@ -145,7 +144,7 @@
&.spectrum-Popover--withTip {
/* tip size minus where it overlaps with popover border */
/* stylelint-disable-next-line csstools/use-logical -- intentional, right stays on the same side even for RTL */
margin-left: calc(var(--mod-popover-pointer-width, var(--spectrum-popover-pointer-width)) - var(--mod-popover-border-width, var(--spectrum-popover-border-width)));
margin-left: calc(var(--mod-popover-pointer-height, var(--spectrum-popover-pointer-height)) - var(--mod-popover-border-width, var(--spectrum-popover-border-width)));
}

/* popover animates towards right ⮕ */
Expand All @@ -162,7 +161,7 @@
&.spectrum-Popover--withTip {
/* tip size minus where it overlaps with popover border */
/* stylelint-disable-next-line csstools/use-logical -- intentional, left stays on the same side even for RTL */
margin-right: calc(var(--mod-popover-pointer-width, var(--spectrum-popover-pointer-width)) - var(--mod-popover-border-width, var(--spectrum-popover-border-width)));
margin-right: calc(var(--mod-popover-pointer-height, var(--spectrum-popover-pointer-height)) - var(--mod-popover-border-width, var(--spectrum-popover-border-width)));
}

/* popover animates towards left ⬅ */
Expand All @@ -178,7 +177,7 @@
/* spacing to include tip in calculation of offset from source */
&.spectrum-Popover--withTip {
/* tip size minus where it overlaps with popover border */
margin-inline-end: calc(var(--mod-popover-pointer-width, var(--spectrum-popover-pointer-width)) - var(--mod-popover-border-width, var(--spectrum-popover-border-width)));
margin-inline-end: calc(var(--mod-popover-pointer-height, var(--spectrum-popover-pointer-height)) - var(--mod-popover-border-width, var(--spectrum-popover-border-width)));
}

/* LTR read, popover animates towards left ⬅ */
Expand All @@ -199,7 +198,7 @@
/* spacing to include tip in calculation of offset from source */
&.spectrum-Popover--withTip {
/* tip size minus where it overlaps with popover border */
margin-inline-start: calc(var(--mod-popover-pointer-width, var(--spectrum-popover-pointer-width)) - var(--mod-popover-border-width, var(--spectrum-popover-border-width)));
margin-inline-start: calc(var(--mod-popover-pointer-height, var(--spectrum-popover-pointer-height)) - var(--mod-popover-border-width, var(--spectrum-popover-border-width)));
}

/* LTR read, popover animates towards right ⮕ */
Expand Down Expand Up @@ -412,3 +411,10 @@
}
}
}

/* windows high contrast mode */
@media (forced-colors: active) {
.spectrum-Popover {
--highcontrast-popover-border-color: CanvasText;
}
}
Loading
Loading