Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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/funky-pears-occur.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@evo-web/marko": patch
Comment thread
LuLaValva marked this conversation as resolved.
---

Confirm & Alert Dialog
1 change: 1 addition & 0 deletions .claude/skills/evo-migrate-marko/SKILL.md
Comment thread
LuLaValva marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ Key changes:
- **Remove `Omit<..., 'on${string}'>`** -- Marko 6 handles events natively via spread.
- **Remove custom `on-*` event props** from the interface -- consumers bind native DOM events directly or use two-way binding callbacks (e.g., `valueChange`, `openChange`, `indexChange`).
- **Use camelCase** for all prop names (not kebab-case).
- **Use `Marko.HTML.*`** for element types -- e.g., `Marko.HTML.H2`, `Marko.HTML.Div`, `Marko.HTML.Button`. Do **not** use `Marko.Input<"h2">` or `Marko.Input<"div">`.
- **Omit only props the component hardcodes** (e.g., `Omit<Marko.HTML.Input, "type" | "role">`).

### HTML attribute pass-through
Expand Down
4 changes: 4 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,10 @@ HTML Semantic Structure → @ebay/skin (CSS/BEM) → Framework Components → In
- ✅ Attribute values containing `>` MUST be wrapped in parentheses: `<const/x=(a > b ? 1 : 0)>`
- ❌ Never: `<const/x=a > b ? 1 : 0>` (the `>` is parsed as the tag close)

**Marko 6 Tag Variable Locality:**
Comment thread
LuLaValva marked this conversation as resolved.

Declare `<const/>`, `<let/>`, `<id/>`, and other tag variables close to where they are first used, not grouped at the top. Exception is variables needed in multiple distant locations.

**React Package Differences:**

- `ebayui-core-react`: Requires `React.forwardRef` wrapper
Expand Down
18 changes: 18 additions & 0 deletions packages/evo-marko/src/tags/evo-alert-dialog/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<h1 style='display: flex; justify-content: space-between; align-items: center;'>
<span>
evo-alert-dialog
</span>
<span style='font-weight: normal; font-size: medium; margin-bottom: -15px;'>
DS vBETA
</span>
</h1>

An alert dialog that forces the user to acknowledge a message before continuing. The dialog can only be dismissed by clicking the confirm button -- Escape and backdrop clicks are blocked.

Uses a native `<dialog>` element with `role="alertdialog"` and `closedby="none"`.

## Examples and Documentation

- [Storybook](https://ebay.github.io/evo-web/evo-marko/?path=/story/navigation-disclosure-evo-alert-dialog)
- [Storybook Docs](https://ebay.github.io/evo-web/evo-marko/?path=/docs/navigation-disclosure-evo-alert-dialog)
- [Code Examples](https://github.com/eBay/evo-web/tree/main/packages/evo-marko/src/tags/evo-alert-dialog/examples)
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
import { buildExtensionTemplate } from "../../common/storybook/utils";
import { type Meta } from "@storybook/marko";
import Readme from "./README.md";
import Component, { type Input } from "./index.marko";
import DefaultTemplate from "./examples/default.marko";
import DefaultCode from "./examples/default.marko?raw";

export default {
title: "navigation & disclosure/evo-alert-dialog",
component: Component,
parameters: {
docs: {
description: {
component: Readme,
},
},
},

argTypes: {
open: {
type: "boolean",
controllable: true,
description: "Whether the alert dialog is open",
table: { defaultValue: { summary: "false" } },
},
header: {
description:
"The header content rendered inside the dialog title (required)",
"@": {
as: {
type: "string",
description:
"The heading element to use for the title. Defaults to `h2`",
},
["<h2> attributes" as any]: {
description:
"All attributes and event handlers from the heading element will be passed through",
},
},
},
confirm: {
description:
"The confirm/acknowledge button (required). Render body is the button label text",
"@": {
["<button> attributes" as any]: {
description:
"All attributes and event handlers from [the native HTML `<button>` tag](https://developer.mozilla.org/en-US/docs/Web/HTML/Reference/Elements/button) will be passed through",
},
},
},
["<dialog> attributes" as any]: {
description:
"All attributes and event handlers from [the native HTML `<dialog>` tag](https://developer.mozilla.org/en-US/docs/Web/HTML/Reference/Elements/dialog) will be passed through",
},
},
} satisfies Meta<Input>;

export const Default = buildExtensionTemplate(DefaultTemplate, DefaultCode);
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import { type Input as AlertDialogInput } from "<evo-alert-dialog>";
export interface Input extends AlertDialogInput {}

<let/open:=input.open>

<evo-button onClick() { open = true; }>
Open Alert Dialog
</evo-button>

<evo-alert-dialog ...input open:=open>
<@header>Alert!</@header>
<@confirm>OK</@confirm>
<p>You must acknowledge this alert to continue.</p>
</evo-alert-dialog>
75 changes: 75 additions & 0 deletions packages/evo-marko/src/tags/evo-alert-dialog/index.marko
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
import { type Input as ButtonInput } from "<evo-button>";

export interface Input extends Omit<Marko.HTML.Dialog, "open" | "closedby" | "role" | "aria-labelledby" | "aria-modal"> {
open?: boolean;
openChange?: (o: boolean) => void;
header: Marko.AttrTag<Marko.HTML.H2 & { as?: string }>;
confirm: Marko.AttrTag<ButtonInput>;
}

<const/{
open: inputOpen,
openChange,
class: inputClass,
header,
confirm,
content,
onAnimationEnd,
...htmlInput
}=input>

<let/open:=input.open>
<id/headerId=header.id>

<script>
if (open && !$dialog().open) {
$dialog().showModal();
}
</script>

<dialog/$dialog
...htmlInput
open=null // tell Marko it's not a part of the spread because the browser does DOM manipulation
role="alertdialog"
aria-modal="true"
closedby="none"
aria-labelledby=headerId
class=[
"dialog",
"dialog--narrow",
!open && "dialog--close",
inputClass,
]
Comment thread
LuLaValva marked this conversation as resolved.
onAnimationEnd(e, el) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Does team have ability to pass their custom animations? If yes, should we add reduced motion to respect user animations. We should discuss this further and add support if needed in following PR.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Custom animations are valid, but if there is ever no animation then we will run into problems. My recommendation to people who have a jarring animation is to make a minimal one under prefers-reduced-motion but not remove it entirely, but in the future we can talk about adding support for no animations

if (!open) {
el.close();
}
onAnimationEnd && onAnimationEnd(e, el);
}
>
<div class="dialog__header">
<const/{ as: headerAs = "h2", ...headerInput }=header>
<${headerAs}
...headerInput
id=headerId
class=["dialog__title", headerInput.class]
/>
</div>
<id/mainId>
<div id=mainId class="dialog__main">
<${content}/>
</div>
<div class="dialog__footer">
<const/{ onClick: confirmOnClick, ...confirmInput }=confirm>
<evo-button
...confirmInput
priority="primary"
autofocus
aria-describedby=mainId
onClick(e, el) {
open = false;
confirmOnClick && confirmOnClick(e, el);
}
/>
</div>
</dialog>
1 change: 1 addition & 0 deletions packages/evo-marko/src/tags/evo-alert-dialog/style.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import "@ebay/skin/dialog";
137 changes: 137 additions & 0 deletions packages/evo-marko/src/tags/evo-alert-dialog/test/test.browser.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
import {
afterAll,
afterEach,
beforeAll,
beforeEach,
describe,
it,
expect,
} from "vitest";
import { render, fireEvent, cleanup } from "@marko/testing-library";
import { userEvent } from "vitest/browser";
import { composeStories } from "@storybook/marko";
import { fastAnimations } from "../../../common/test-utils/index";
import * as stories from "../alert-dialog.stories";

const { Default } = composeStories(stories);

beforeAll(() => fastAnimations.start());
afterAll(() => fastAnimations.stop());
afterEach(cleanup);

let component: Awaited<ReturnType<typeof render>>;

describe("evo-alert-dialog", () => {
describe("given the dialog is in the default (closed) state", () => {
beforeEach(async () => {
component = await render(Default);
});

it("should render with a dialog element", () => {
expect(component.container.querySelector("dialog")).toBeTruthy();
});

it("should have the dialog class", () => {
const dialog = component.container.querySelector("dialog");
expect(dialog?.classList.contains("dialog")).toBe(true);
});

it("should have dialog--close class when not open", () => {
const dialog = component.container.querySelector("dialog");
expect(dialog?.classList.contains("dialog--close")).toBe(true);
});

it("should have dialog--narrow class", () => {
const dialog = component.container.querySelector("dialog");
expect(dialog?.classList.contains("dialog--narrow")).toBe(true);
});
});

describe("given the dialog is in the open state", () => {
beforeEach(async () => {
component = await render(Default, { open: true });
});

it("should render the dialog element", () => {
const dialog = component.container.querySelector("dialog");
expect(dialog).toBeTruthy();
});

it("should not have dialog--close class when open", () => {
const dialog = component.container.querySelector("dialog");
expect(dialog?.classList.contains("dialog")).toBe(true);
expect(dialog?.classList.contains("dialog--close")).toBe(false);
});

it("should have role alertdialog", () => {
const dialog = component.container.querySelector("dialog");
expect(dialog?.getAttribute("role")).toBe("alertdialog");
});

it("should have aria-modal set to true", () => {
const dialog = component.container.querySelector("dialog");
expect(dialog?.getAttribute("aria-modal")).toBe("true");
});

it("should have closedby set to none", () => {
const dialog = component.container.querySelector("dialog");
expect(dialog?.getAttribute("closedby")).toBe("none");
});

it("should render header with h2 by default", () => {
const title = component.container.querySelector(".dialog__title");
expect(title).toBeTruthy();
expect(title?.tagName.toLowerCase()).toBe("h2");
});

it("should link dialog to header via aria-labelledby", () => {
const dialog = component.container.querySelector("dialog");
const title = component.container.querySelector(".dialog__title");
const titleId = title?.id;
expect(titleId).toBeTruthy();
expect(dialog?.getAttribute("aria-labelledby")).toBe(titleId);
});

it("should render the confirm button", () => {
const button = component.getByRole("button", { name: "OK" });
expect(button).toBeTruthy();
});

it("should have aria-describedby on the confirm button referencing main content", () => {
const button = component.getByRole("button", { name: "OK" });
const describedBy = button.getAttribute("aria-describedby");
expect(describedBy).toBeTruthy();
const mainEl = document.getElementById(describedBy!);
expect(mainEl).toBeTruthy();
expect(mainEl?.textContent).toContain(
"You must acknowledge this alert to continue.",
);
});

it("should render main content area", () => {
const main = component.container.querySelector(".dialog__main");
expect(main).toBeTruthy();
});

describe("when the confirm button is clicked", () => {
it("should close the dialog", async () => {
await fireEvent.click(
component.getByRole("button", { name: "OK" }),
);
const dialog = component.container.querySelector("dialog");
expect(dialog?.classList.contains("dialog--close")).toBe(true);
});
});

describe("when Escape key is pressed", () => {
it("should not close the dialog", async () => {
const user = userEvent.setup();
const button = component.getByRole("button", { name: "OK" });
button.focus();
await user.keyboard("{Escape}");
const dialog = component.container.querySelector("dialog");
expect(dialog?.classList.contains("dialog--close")).toBe(false);
});
});
});
});
16 changes: 16 additions & 0 deletions packages/evo-marko/src/tags/evo-alert-dialog/test/test.server.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import { describe, it } from "vitest";
import { composeStories } from "@storybook/marko";
import { snapshotHTML } from "../../../common/test-utils/snapshots";
import * as stories from "../alert-dialog.stories";

const { Default } = composeStories(stories);

describe("evo-alert-dialog SSR", () => {
it("renders default (closed)", async () => {
await snapshotHTML(Default);
});

it("renders in open state", async () => {
await snapshotHTML(Default, { open: true });
});
});
18 changes: 18 additions & 0 deletions packages/evo-marko/src/tags/evo-confirm-dialog/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<h1 style='display: flex; justify-content: space-between; align-items: center;'>
<span>
evo-confirm-dialog
</span>
<span style='font-weight: normal; font-size: medium; margin-bottom: -15px;'>
DS vBETA
</span>
</h1>

A confirm dialog that forces the user to make a choice to either confirm or reject. The dialog can be dismissed by clicking one of the two buttons, and pressing Escape triggers the reject action.

Uses a native `<dialog>` element with `role="alertdialog"` and `closedby="closerequest"`.

## Examples and Documentation

- [Storybook](https://ebay.github.io/evo-web/evo-marko/?path=/story/navigation-disclosure-evo-confirm-dialog)
- [Storybook Docs](https://ebay.github.io/evo-web/evo-marko/?path=/docs/navigation-disclosure-evo-confirm-dialog)
- [Code Examples](https://github.com/eBay/evo-web/tree/main/packages/evo-marko/src/tags/evo-confirm-dialog/examples)
Loading
Loading