Skip to content

chore(popover): adjust/enhance popover spacing by default [CSS-845] #4174

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

Merged
castastrophe merged 1 commit into from
Aug 29, 2025
Merged

chore(popover): adjust/enhance popover spacing by default [CSS-845] #4174

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/modern-times-happen.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@spectrum-css/popover": patch
---

Popover positioning "top-left" and "top-right" were using inline properties which caused the spacings to swap in RTL mode. This was unintended as the "right" and "left" naming conventions are meant to retain their location in LTR or RTL mode.
2 changes: 0 additions & 2 deletions components/actionbar/stories/template.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,6 @@ export const Template = ({
],
}, context),
],
popoverHeight: 42,
Copy link
Contributor Author

@castastrophe castastrophe Aug 28, 2025

Choose a reason for hiding this comment

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

No longer needed! This is calculated now on the fly (and more accurately) by the popover wrapper.

popoverWidth: 500,
}, context)}
</div>
`;
Expand Down
4 changes: 2 additions & 2 deletions components/actionmenu/stories/template.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,10 @@ export const Template = ({
iconName,
iconSet,
id: triggerId,
customClasses,
}, context),
position: "bottom-start",
customStyles,
customWrapperClasses: customClasses,
customWrapperStyles: customStyles,
content: [
(passthroughs) => Menu({
...passthroughs,
Expand Down
15 changes: 0 additions & 15 deletions components/alertdialog/stories/alertdialog.stories.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,6 @@ export default {
type: "figma",
url: "https://www.figma.com/design/Mngz9H7WZLbrCvGQf3GnsY/S2---Desktop?node-id=21917-157",
},
docs: {
story: {
height: "300px",
}
},
packageJson,
metadata,
status: {
Expand Down Expand Up @@ -119,11 +114,6 @@ Information.args = {
content: "If you enjoy our app, would you mind taking a moment to rate it?",
};
Information.parameters = {
docs: {
story: {
height: "400px",
}
},
chromatic: { disableSnapshot: true },
};

Expand Down Expand Up @@ -187,11 +177,6 @@ Overflow.args = {
};
Overflow.parameters = {
chromatic: { disableSnapshot: true },
docs: {
story: {
height: "525px",
},
},
};

/**
Expand Down
4 changes: 4 additions & 0 deletions components/coachindicator/stories/template.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { getRandomId } from "@spectrum-css/preview/decorators";
import { html } from "lit-html";
import { classMap } from "lit-html/directives/class-map.js";
import { ifDefined } from "lit-html/directives/if-defined.js";
import { styleMap } from "lit-html/directives/style-map.js";

import "../index.css";
Expand All @@ -10,6 +12,7 @@ export const Template = ({
staticColor,
customClasses = [],
customStyles = {},
id = getRandomId("coach-indicator"),
} = {}) => {
return html`
<div
Expand All @@ -21,6 +24,7 @@ export const Template = ({
...customClasses.reduce((a, c) => ({ ...a, [c]: true }), {}),
})}
style=${styleMap(customStyles)}
id=${ifDefined(id)}
>
${Array.from({ length: 3 }).map(() => html`
<div class=${classMap({ [`${rootClass}-ring`]: true })}></div>
Expand Down
20 changes: 0 additions & 20 deletions components/coachmark/stories/coachmark.stories.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,11 +94,6 @@ export default {
},
packageJson,
metadata,
docs: {
story: {
height: "525px",
},
},
status: {
type: "migrated",
},
Expand All @@ -121,11 +116,6 @@ Standard.parameters = {
chromatic: {
disableSnapshot: true,
},
docs: {
story: {
height: "475px",
},
},
};
Standard.args = {
imageSource: "example-card-landscape.png",
Expand All @@ -141,11 +131,6 @@ StandardNoMedia.parameters = {
chromatic: {
disableSnapshot: true,
},
docs: {
story: {
height: "250px",
},
},
};

/** Images and media have a minimum height and can grow with the parent component. Fixed height media is constrained to a 4:3 aspect ratio by applying the `spectrum-CoachMark-image-wrapper--fixedHeight` class. When this fixed height class is used, the height can be customized using the modifiable custom property `--mod-coachmark-media-fixed-height`. */
Expand All @@ -155,11 +140,6 @@ MediaOptions.args = {
imageSource: "example-card-portrait.png",
};
MediaOptions.parameters = {
docs: {
story: {
height: "725px",
},
},
chromatic: {
disableSnapshot: true,
},
Expand Down
100 changes: 43 additions & 57 deletions components/coachmark/stories/template.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,32 +47,34 @@ export const CoachContainer = (
</div>
`,
)}
<div class="spectrum-CoachMark-header">
<div class="spectrum-CoachMark-header" style=${styleMap({
"--mod-popover-width": "0px",
"--mod-popover-height": "0px",
"--mod-popover-wrapper-spacing": "0px",
})}>
<div class="spectrum-CoachMark-title">${title}</div>
${when(
hasActionMenu,
() =>
html` <div class="spectrum-CoachMark-action-menu">
${ActionMenu(
() => ActionMenu(
{
isOpen,
position: "bottom-start",
iconName: "More",
size: scale === "large" ? "s" : "m",
customClasses: [
`${rootClass}-action-menu`
],
items: [
{
label: "Skip tour",
},
{
isOpen,
position: "bottom-start",
iconName: "More",
size: scale === "large" ? "s" : "m",
items: [
{
label: "Skip tour",
},
{
label: "Reset tour",
},
],
popoverHeight: 68,
popoverWidth: 84,
label: "Reset tour",
},
context,
)}
</div>`,
],
},
context,
),
)}
</div>
<div class="spectrum-CoachMark-content">
Expand Down Expand Up @@ -127,29 +129,24 @@ export const CoachContainer = (
};

export const Template = (args, context) => {
return html`
<div
class=${classMap({
[args.rootClass]: true,
...args.customClasses.reduce((a, c) => ({ ...a, [c]: true }), {}),
})}
style=${styleMap(args.customStyles)}
>
${Popover(
{
customStyles: {
"inline-size": "var(--spectrum-coach-mark-width)",
},
customClasses: [`${args.rootClass}-popover`],
isOpen: true,
position: "right-top",
trigger: (passthrough) => CoachIndicator(passthrough, context),
content: [CoachContainer(args, context)],
},
context,
)}
</div>
`;
return Popover(
{
customWrapperClasses: [
args.rootClass,
...args?.customClasses ?? []
],
customStyles: {
...args?.customStyles ?? {},
"inline-size": "var(--spectrum-coach-mark-width)",
},
customClasses: [`${args.rootClass}-popover`],
isOpen: true,
position: "right-top",
trigger: (passthrough) => CoachIndicator(passthrough, context),
content: [CoachContainer(args, context)],
},
context,
);
};

/* Displays open and closed action menus in a single story. */
Expand All @@ -165,15 +162,7 @@ export const CoachmarkMenuStatesTemplate = (args, context) =>
Container({
withBorder: false,
heading: "With action menu (closed) and media",
content: Template(
{
...args,
customStyles: {
"height": "265px"
}
},
context,
),
content: Template(args, context),
}),
Container({
withBorder: false,
Expand All @@ -183,10 +172,7 @@ export const CoachmarkMenuStatesTemplate = (args, context) =>
...args,
hasImage: false,
hasActionMenu: true,
isOpen: true,
customStyles: {
"height": "260px"
}
isOpen: true
},
context,
),
Expand Down
5 changes: 0 additions & 5 deletions components/combobox/stories/combobox.stories.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,11 +211,6 @@ DefaultGroup.args = {
DefaultGroup.tags = ["!dev"];
DefaultGroup.parameters = {
chromatic: { disableSnapshot: true },
docs: {
story: {
height: "360px",
},
},
};

/**
Expand Down
3 changes: 0 additions & 3 deletions components/combobox/stories/combobox.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,6 @@ export const ComboBoxGroup = Variants({
{
testHeading: "Open",
isOpen: true,
wrapperStyles: {
"min-block-size": "250px",
},
},
{
testHeading: "Help text with label",
Expand Down
9 changes: 1 addition & 8 deletions components/combobox/stories/template.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,6 @@ export const Template = ({
}, 0);
}

const popoverHeight = size === "s" ? 106 : size === "l" ? 170 : size === "xl" ? 229 : 142; // default value is "m"
const adjustedPopoverHeight = showFieldLabel ? popoverHeight : popoverHeight + 32; // Subtract label height when no label

return html`
<div
class=${classMap({
Expand All @@ -78,10 +75,7 @@ export const Template = ({
})}
id=${ifDefined(id)}
data-testid=${ifDefined(testId ?? id)}
style=${styleMap({
...customStyles,
["margin-block-end"]: !isReadOnly && isOpen && !isDisabled ? `${adjustedPopoverHeight}px` : undefined,
})}
style=${styleMap(customStyles)}
role="combobox"
aria-expanded=${isOpen}
aria-haspopup="listbox"
Expand Down Expand Up @@ -150,7 +144,6 @@ export const Template = ({
"inline-size": size === "s" ? "192px" : size === "l" ? "224px" : size === "xl" ? "240px" : "208px",
},
content,
popoverHeight,
}, context)}
${when(helpText, () =>
HelpText({
Expand Down
Loading
Loading