diff --git a/.changeset/three-geese-relax.md b/.changeset/three-geese-relax.md new file mode 100644 index 0000000000..de375e3e10 --- /dev/null +++ b/.changeset/three-geese-relax.md @@ -0,0 +1,7 @@ +--- +"@spectrum-css/popover": patch +--- + +fix(popover): correct left/right tips for RTL + +Includes exposing Start and End tip placement variants in Storybook, and increased VRT coverage for Chromatic. diff --git a/components/popover/index.css b/components/popover/index.css index a23941955d..2b58ba45a2 100644 --- a/components/popover/index.css +++ b/components/popover/index.css @@ -150,7 +150,8 @@ governing permissions and limitations under the License. /* 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))); + /* 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))); } /* popover animates towards right ⮕ */ @@ -166,7 +167,8 @@ governing permissions and limitations under the License. /* 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))); + /* 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))); } /* popover animates towards left ⬅ */ @@ -342,7 +344,9 @@ governing permissions and limitations under the License. &.spectrum-Popover--left-bottom, &.spectrum-Popover--left-top { .spectrum-Popover-tip { - inset-inline: 100% auto; + /* stylelint-disable-next-line csstools/use-logical -- intentional, left stays on the same side even for RTL */ + left: 100%; + right: auto; } } @@ -351,7 +355,9 @@ governing permissions and limitations under the License. &.spectrum-Popover--right-bottom, &.spectrum-Popover--right-top { .spectrum-Popover-tip { - inset-inline: auto 100%; + /* stylelint-disable-next-line csstools/use-logical -- intentional, right stays on the same side even for RTL */ + right: 100%; + left: auto; /* flip tip to point left ◁ */ transform: scaleX(-1); diff --git a/components/popover/stories/popover.stories.js b/components/popover/stories/popover.stories.js index 0990c12323..37c446b03f 100644 --- a/components/popover/stories/popover.stories.js +++ b/components/popover/stories/popover.stories.js @@ -1,11 +1,38 @@ +import { Template as Typography } from "@spectrum-css/typography/stories/template.js"; import { userEvent, within } from "@storybook/testing-library"; import { html } from "lit"; +import { when } from "lit/directives/when.js"; import { Template } from "./template"; import { Template as ActionButton } from "@spectrum-css/actionbutton/stories/template.js"; import { Template as Menu } from "@spectrum-css/menu/stories/template.js"; +const placementOptions = [ + "top", + "top-left", + "top-right", + "top-start", + "top-end", + "bottom", + "bottom-left", + "bottom-right", + "bottom-start", + "bottom-end", + "right", + "right-bottom", + "right-top", + "left", + "left-bottom", + "left-top", + "start", + "start-top", + "start-bottom", + "end", + "end-top", + "end-bottom", +]; + /** * A popover is used to display transient content (menus, options, additional actions etc.) and appears when clicking/tapping on a source (tools, buttons, etc.). It stands out via its visual style (stroke and drop shadow) and floats on top of the rest of the interface. */ @@ -44,20 +71,7 @@ export default { category: "Component", }, control: "select", - options: [ - "top", - "top-start", - "top-end", - "bottom", - "bottom-start", - "bottom-end", - "left", - "left-top", - "left-bottom", - "right", - "right-top", - "right-bottom", - ], + options: placementOptions, if: { arg: "nested", truthy: false }, }, }, @@ -113,10 +127,59 @@ export default { }, }, decorators: [ - (Story, context) => html`
${Story(context)}
` + (Story, context) => html` + +
${Story(context)}
+ `, ], }; + +const ChromaticTipPlacementVariants = (args) => html` + ${placementOptions.map(option => { + const optionDescription = () => { + if (option.startsWith("start") || option.startsWith("end")) + return "Changes side with text direction (like a logical property)"; + if (option.startsWith("left") || option.startsWith("right")) + return "Text direction does not affect the position"; + return null; + }; + + return html` +
+ ${Typography({ + semantics: "detail", + size: "l", + content: [`${option}`], + })} +
+ ${when(optionDescription() !== null, () => html` + ${Typography({ + semantics: "detail", + size: "s", + content: [`${optionDescription()}`], + })} + `)} +
+
+ ${Template({ + ...args, + position: option, + isOpen: true, + trigger: () => null, + })} +
+
+ `; + })} +`; + export const Default = Template.bind({}); Default.play = async ({ canvasElement }) => { const canvas = within(canvasElement); @@ -124,10 +187,12 @@ Default.play = async ({ canvasElement }) => { }; Default.args = {}; -export const WithTip = Template.bind({}); +export const WithTip = (args) => html` + ${window.isChromatic() ? ChromaticTipPlacementVariants(args) : Template(args)} +`; WithTip.play = async ({ canvasElement }) => { const canvas = within(canvasElement); - await userEvent.click(canvas.getByRole("button")); + window.isChromatic() ? null : await userEvent.click(canvas.getByRole("button")); }; WithTip.args = { withTip: true, diff --git a/components/popover/stories/template.js b/components/popover/stories/template.js index 104eadf62c..6eb9366c89 100644 --- a/components/popover/stories/template.js +++ b/components/popover/stories/template.js @@ -60,49 +60,61 @@ export const Template = ({ const popWidth = popover.offsetWidth ?? 0; const popHeight = popover.offsetHeight ?? 0; const textDir = getComputedStyle(document.querySelector("html")).direction; + let x, y; let xOffset = "+ 0px"; let yOffset = "+ 0px"; - if (position.includes("top") || position.includes("bottom") && !(position.includes("-top") || position.includes("-bottom"))) { + + if (position.startsWith("top") || position.startsWith("bottom")) { x = triggerXCenter - (popWidth > 0 ? popWidth / 2 : popWidth); } - if (position.includes("left") || position.includes("right")) { + if (position.includes("left") || position.includes("right") || position.startsWith("start") || position.startsWith("end")) { y = triggerYCenter - (popHeight > 0 ? popHeight / 2 : popHeight); } - if (position.includes("top") && !position.includes("-top")) { + if (position.startsWith("top")) { y = rect.top - popHeight; yOffset = withTip ? "- (var(--spectrum-popover-pointer-height) + var(--spectrum-popover-animation-distance) - 1px)" : "- var(--spectrum-popover-animation-distance)"; } - else if (position.includes("bottom") && !position.includes("-bottom")) { + else if (position.startsWith("bottom")) { y = rect.bottom; yOffset = "+ (var(--spectrum-popover-animation-distance))"; } - else if (position.includes("left")) { + else if (position.includes("left")) { if (textDir == "rtl") { x = rect.right; xOffset = withTip ? "+ 0px" : "+ var(--spectrum-popover-animation-distance)"; } - else { + else { x = rect.left - popWidth; xOffset = withTip ? "- ((var(--spectrum-popover-pointer-width) / 2) + var(--spectrum-popover-animation-distance) - 2px)" : "- var(--spectrum-popover-animation-distance)"; } } - else if (position.includes("right")) { + else if (position.includes("right")) { if (textDir == "rtl") { x = rect.left - popWidth; xOffset = withTip ? "- ((var(--spectrum-popover-pointer-width) / 2) + var(--spectrum-popover-animation-distance) - 2px)" : "- var(--spectrum-popover-animation-distance)"; } - else { + else { x = rect.right; xOffset = withTip ? "+ 0px" : "+ var(--spectrum-popover-animation-distance)"; } } + else if (position.includes("start")) { + x = rect.left - popWidth; + xOffset = withTip + ? "- ((var(--spectrum-popover-pointer-width) / 2) + var(--spectrum-popover-animation-distance) - 2px)" + : "- var(--spectrum-popover-animation-distance)"; + } + else if (position.includes("end")) { + x = rect.right; + xOffset = withTip ? "+ 0px" : "+ var(--spectrum-popover-animation-distance)"; + } if (x) transforms.push(`translateX(calc(var(--flow-direction) * calc(${parseInt(x, 10)}px ${xOffset})))`); if (y) transforms.push(`translateY(calc(${y}px ${yOffset}))`); @@ -111,13 +123,13 @@ export const Template = ({ if (position === "top-start" || position === "bottom-start") { additionalStyles["inset-inline-start"] = "calc(" + (popWidth / 2) + "px - var(--spectrum-popover-pointer-edge-offset))"; } - else if (position === "top-end" || position === "bottom-end") { + else if (position === "top-end" || position === "bottom-end") { additionalStyles["inset-inline-start"] = "calc(-1 *" + (popWidth / 2) + "px + var(--spectrum-popover-pointer-edge-offset))"; } - else if (position === "left-top" || position === "right-top") { + else if (position === "left-top" || position === "right-top" || position === "start-top" || position === "end-top") { additionalStyles["inset-block-start"] = "calc(" + (popHeight / 2) + "px - var(--spectrum-popover-pointer-edge-offset))"; } - else if (position === "left-bottom" || position === "right-bottom") { + else if (position === "left-bottom" || position === "right-bottom" || position === "start-bottom" || position === "end-bottom") { additionalStyles["inset-block-start"] = "calc(-1 *" + (popHeight / 2) + "px + var(--spectrum-popover-pointer-edge-offset))"; }