From 78845024ee6c642c1f2fed24f281dcea5856391f Mon Sep 17 00:00:00 2001 From: Stewest Date: Sat, 28 Mar 2026 12:22:56 +1300 Subject: [PATCH 1/9] planing 30m and Initial iteration. 1.5 hours --- README.md | 107 +++++++++++++++++------- package.json | 2 +- src/Component/Dialog/Dialog.stories.ts | 55 ++++++++++++ src/Component/Dialog/Elements/Dialog.ts | 82 ++++++++++++++++++ src/Component/Dialog/dialog.css | 64 ++++++++++++++ src/Component/Dialog/dialog.entry.js | 1 + src/Component/Dialog/dialog.twig | 17 ++++ src/elements.ts | 18 ++-- 8 files changed, 305 insertions(+), 41 deletions(-) create mode 100644 src/Component/Dialog/Dialog.stories.ts create mode 100644 src/Component/Dialog/Elements/Dialog.ts create mode 100644 src/Component/Dialog/dialog.css create mode 100644 src/Component/Dialog/dialog.entry.js create mode 100644 src/Component/Dialog/dialog.twig diff --git a/README.md b/README.md index 494194f..d402a98 100644 --- a/README.md +++ b/README.md @@ -2,55 +2,100 @@ ## Overview -We'd like you to implement a modal dialog component using the codebase provided. We've kept the brief intentionally -loose in places - we're interested in the decisions you make, not just the end result. +Implement a modal dialog component using the codebase provided. ## The task Using the existing codebase as your starting point, implement a modal dialog component. A rough reference for behaviour and appearance can be found here: [stripe.com](https://stripe.com/au) (see expand icon on homepage cards). -Consider this a starting point, not a strict spec. Your implementation should fit the patterns and conventions already -established in the codebase. +Implementation should fit the patterns and conventions already established in the codebase. -## Requirements +## What to submit -The dialog should: +- [x] See the [CONTRIBUTING.md](./CONTRIBUTING.md) file for setup instructions and guidelines. -- Open and close correctly -- Avoid using any external UI libraries - we'd like to see your own implementation -- Be keyboard accessible - including Escape to close, and correct focus management when opening and closing -- Follow the existing component patterns, naming conventions, and file structure in the codebase -- Be written in TypeScript +## Process -## What we're not prescribing +- [x] Read through task 10:30 and plan +- [x] Onboard & check + - [x] pnpm audit - (PMG) notes critical npm packages: look at --fix +- [x] Plan 30m and Discovery + - [x] need to install oxc.oxc-vscode formatter to match code styles. -We've deliberately left the following open - please make your own decisions and note them down: +- [x] Run tests on current code. + - [x] `pnpm test` - worked - [x] `pnpm lint` - Found 40 errors in 4 files. -- Mobile behaviour and breakpoints -- What happens to page scroll when the dialog is open -- Animation and transition behaviour -- How the trigger element is handled +``` + Errors Files + 10 src/elements.ts:3 + 1 src/Utility/Elements/breakpoint-loader.ts:75 + 1 src/Utility/Elements/io-loader.ts:58 + 28 src/Utility/Elements/keyboard.ts:7 +``` -## What to submit +- [x] `pnpm test` -Along with your code, please include a brief README (a few bullet points is fine) covering: +``` + Snapshots 1 failed + Test Files 11 failed | 28 passed (39) + Tests 57 failed | 54 passed (111) +``` -- Any assumptions you made where the spec was unclear -- Any trade-offs or decisions you'd approach differently with more time -- Anything you noticed in the existing codebase you'd flag in a code review +Deciding to move on... + +- find components and read through current component/theme implementations +- [x] Stripe example Components used + - [x] Buttons with arrow icon and links + - [x] Expand / Close icons + - [x] Grids: 2/3, 1/3, full + - [x] Cards and backgrounds + - [x] Text, quotes, logo, headings + +- [x] Ideation 30m - decide on way forward + - [x] Can I use dialog element? + - [x] But if the implementation should be written in TypeScript - then change to div and custom open/close. + +- [x] Iterate + - [x] Test assumptions + - [x] Create storybook component + - [] Make a Story with multiple dialogs to test open / close of each on a page. + - [ ] Write tests + docs + - [ ] commits -## Time +## Requirement checklist -We'd suggest around 2–3 hours, but there's no hard limit. We're more interested in quality and thoughtfulness than -completeness - if you run out of time, notes on what you'd do next are just as valuable. +The dialog should: + +- [] Open and close correctly +- [] Avoid using any external UI libraries - we'd like to see your own implementation +- [] Be keyboard accessible - including Escape to close, and correct focus management when opening and closing +- [] Follow the existing component patterns, naming conventions, and file structure in the codebase +- [] Be written in TypeScript + +- [] Mobile behaviour and breakpoints +- [] I'd want to fix main background, so scroll only affects the content of the dialog when open +- [] Animation and transition behaviour +- [] How the trigger element is handled + +- [] Run tests, linters +- [] Docs +- [] Commit -## Follow-up +## Submission + +- Any assumptions you made where the spec was unclear + - [x] whether to use dialog element + - https://caniuse.com/?search=dialog + - https://developer.mozilla.org/en-US/docs/Web/HTML/Reference/Elements/dialog > Global 96.86% + - [] or latest popover with polyfills for browsers not yet compatible. + - [x] But suggestion is to use TypeScript, so will do a more manual custom dialog + +- Any trade-offs or decisions you'd approach differently with more time -We'll schedule a short call to walk through your submission together. Be prepared to talk through your decisions - there -are no trick questions, we just want to understand your thinking. +- [x] Anything you noticed in the existing codebase you'd flag in a code review + -- NPM packages security audit - 1 critical package -## Ready to start? +- [x] include a README (this): -Great! Please see the [CONTRIBUTING.md](./CONTRIBUTING.md) file for setup instructions and guidelines on how to submit -your work. We look forward to seeing your implementation! +https://github.com/stewest/frontend-challenge diff --git a/package.json b/package.json index b4ec425..b20a1cd 100644 --- a/package.json +++ b/package.json @@ -78,5 +78,5 @@ "engines": { "node": ">= 22.0" }, - "packageManager": "pnpm@10.32.1" + "packageManager": "pnpm@10.33.0" } diff --git a/src/Component/Dialog/Dialog.stories.ts b/src/Component/Dialog/Dialog.stories.ts new file mode 100644 index 0000000..5ec5aa5 --- /dev/null +++ b/src/Component/Dialog/Dialog.stories.ts @@ -0,0 +1,55 @@ +import { Meta, StoryObj } from "@storybook/html-vite" +import Component from "./dialog.twig" +import Heading from "../../Atom/Heading/heading.twig" +import "./Elements/Dialog" +import "./dialog.css" +import { Heading as HeadingType, HeadingTypes, WysiwygText } from "@pnx-mixtape/ids-shape" + +export type Dialog = { + title?: HeadingType + toggleAll?: boolean + content: WysiwygText + open?: boolean + id?: string +} + +const meta: Meta = { + tags: ["autodocs", "ids-mvp"], + component: Component, + args: { + title: Heading({ + title: "This is a HTML Dialog Element", + as: HeadingTypes.TWO, + }), + content: "This is the default story content text", + }, + argTypes: { + title: { + description: + "Optional [Heading](/?path=/docs/atom-heading--docs) component, displayed above the Dialog.", + control: "text", + }, + content: { + description: "Add text for the dialog", + control: "text", + }, + }, +} + +export default meta +type Story = StoryObj + +export const Dialog: Story = {} + +export const DefaultOpen: Story = { + args: { + ...meta.args, + content: "This is the default story content text", + }, +} + +export const ToggleAll: Story = { + args: { + toggleAll: true, + }, +} diff --git a/src/Component/Dialog/Elements/Dialog.ts b/src/Component/Dialog/Elements/Dialog.ts new file mode 100644 index 0000000..7e7bdd6 --- /dev/null +++ b/src/Component/Dialog/Elements/Dialog.ts @@ -0,0 +1,82 @@ +/** + * Dialog + * @file Support opening on hash, adding an ID attribute and toggling on print. + */ + +import { makeAnchor } from "../../../Utility/utilities" + +export default class Dialog extends HTMLElement { + internals_: ElementInternals + controller: AbortController + + constructor() { + super() + this.internals_ = this.attachInternals() + this.controller = new AbortController() + } + + connectedCallback(): void { + if (!this.dialog || !this.trigger) return + + const { signal }: AbortController = this.controller + document.addEventListener("beforeprint", this.handleOpen, { + signal, + }) + document.addEventListener("afterprint", this.handleClose, { + signal, + }) + this.handleHash() + document.addEventListener("hashchange", this.handleHash, { signal }) + } + + disconnectedCallback(): void { + this.controller.abort() + } + + handleOpen = (): void => { + if (!this.dialog) return + this.dialog.open = true + } + + handleClose = (): void => { + if (!this.dialog) return + this.dialog.open = false + } + + handleHash = (): void => { + const { hash }: Location = window.location + if (hash && hash === `#${this.dialog?.id}`) { + this.handleOpen() + } + } + + get dialog(): HTMLDialogElement | null { + const dialog: HTMLDialogElement | null = this.querySelector("dialog") + if (!dialog) { + throw new Error(`${this.localName} must contain a element.`) + } + dialog.id = dialog.id || this.generatedId() + return dialog + } + + get trigger(): HTMLElement | null { + const trigger: HTMLElement | null = this.querySelector("summary") + if (!trigger) { + throw new Error(`${this.localName} must contain a element.`) + } + return trigger + } + + generatedId = (): string => { + const string: string | undefined = this.trigger?.textContent?.trim() + return !string ? "" : makeAnchor(string) + } +} + +if (!customElements.get("mx-dialog")) customElements.define("mx-dialog", Dialog) + +declare global { + interface HTMLElementTagNameMap { + "mx-dialog": Dialog + } +} diff --git a/src/Component/Dialog/dialog.css b/src/Component/Dialog/dialog.css new file mode 100644 index 0000000..884820f --- /dev/null +++ b/src/Component/Dialog/dialog.css @@ -0,0 +1,64 @@ +/** + * Dialog + */ + +@layer design-system.defaults { + dialog { + position: relative; + } + + :is(mx-dialog) { + display: block; + } +} + +@layer design-system.components { + .mx-dialog__title { + margin-block-end: var(--spacing-m); + display: flex; + flex-flow: row wrap; + gap: var(--spacing-s); + align-items: center; + } + + .mx-dialog { + & .mx-dialog__toggle { + cursor: pointer; + inline-size: 100%; + } + + &:is(> h2, > h3, > h4, > h5, > h6) { + all: unset; + } + + & > .mx-dialog__content { + display: none; + opacity: 0; + overflow: hidden; + } + + &:is([open], [data-open="true"]) { + & > .mx-dialog__content { + opacity: 1; + display: block; + + @starting-style { + opacity: 0; + display: block; + } + } + } + } +} + +/** + * Print stylesheet + */ + +@media print { + .mx-dialog { + & .mx-dialog__content { + display: block !important; + } + } +} diff --git a/src/Component/Dialog/dialog.entry.js b/src/Component/Dialog/dialog.entry.js new file mode 100644 index 0000000..e25246b --- /dev/null +++ b/src/Component/Dialog/dialog.entry.js @@ -0,0 +1 @@ +import './Elements/Dialog' diff --git a/src/Component/Dialog/dialog.twig b/src/Component/Dialog/dialog.twig new file mode 100644 index 0000000..44859a5 --- /dev/null +++ b/src/Component/Dialog/dialog.twig @@ -0,0 +1,17 @@ +{% set baseClass = 'mx-dialog' %} +{% set classes = [ + baseClass, +] %} +{% set attributes = (attributes ?? create_attribute()).addClass(classes) %} + + + {# uses HTML Dialog Element #} + + + +

{{ title }}

+ {{ content }} + + + + diff --git a/src/elements.ts b/src/elements.ts index 664db52..4fd8688 100644 --- a/src/elements.ts +++ b/src/elements.ts @@ -1,13 +1,13 @@ export { default as Accordion } from "./Component/Accordion/Elements/Accordion" export { default as AccordionGroup } from "./Component/Accordion/Elements/AccordionGroup" -export { default as AccordionMobile } from "./Component/Accordion/Elements/AccordionMobile" -export { default as AccordionDiv } from "./Component/Accordion/Elements/AccordionDiv" +// export { default as AccordionMobile } from "./Component/Accordion/Elements/AccordionMobile" +// export { default as AccordionDiv } from "./Component/Accordion/Elements/AccordionDiv" export { default as Dialog } from "./Component/Dialog/Elements/Dialog" -export { default as DropMenu } from "./Component/DropMenu/Elements/DropMenu" -export { default as ClosableAlert } from "./Component/GlobalAlert/Elements/ClosableAlert" -export { default as Sticky } from "./Component/Sticky/Elements/Sticky" -export { default as Tabs } from "./Component/Tabs/Elements/Tabs" -export { default as GlobalToggle } from "./Layout/Header/Elements/GlobalToggle" -export { default as InPageNavigation } from "./Component/InPageNavigation/Elements/InPageNavigation" -export { default as Navigation } from "./Component/Navigation/Elements/Navigation" +// export { default as DropMenu } from "./Component/DropMenu/Elements/DropMenu" +// export { default as ClosableAlert } from "./Component/GlobalAlert/Elements/ClosableAlert" +// export { default as Sticky } from "./Component/Sticky/Elements/Sticky" +// export { default as Tabs } from "./Component/Tabs/Elements/Tabs" +// export { default as GlobalToggle } from "./Layout/Header/Elements/GlobalToggle" +// export { default as InPageNavigation } from "./Component/InPageNavigation/Elements/InPageNavigation" +// export { default as Navigation } from "./Component/Navigation/Elements/Navigation" export * from "./Utility/utilities" From ea82b5e08eeb0703b1dd0610c6eeb0477107d00e Mon Sep 17 00:00:00 2001 From: Stewest Date: Sat, 28 Mar 2026 14:50:11 +1300 Subject: [PATCH 2/9] 3h30mark added cusotm dialog but stuck --- README.md | 9 ++-- src/Component/Dialog/Dialog.stories.ts | 19 ++++---- src/Component/Dialog/Elements/Dialog.ts | 58 ++++++++++++++++--------- src/Component/Dialog/dialog.css | 21 +++++---- src/Component/Dialog/dialog.twig | 23 ++++++---- 5 files changed, 78 insertions(+), 52 deletions(-) diff --git a/README.md b/README.md index d402a98..4b8fe1b 100644 --- a/README.md +++ b/README.md @@ -59,7 +59,10 @@ Deciding to move on... - [x] Iterate - [x] Test assumptions - [x] Create storybook component + - [] Style a Story. + - [] Add modifiers for Dialog storybook component - [] Make a Story with multiple dialogs to test open / close of each on a page. + - [] Style dialogs on a page and ::backdrop. - [ ] Write tests + docs - [ ] commits @@ -68,10 +71,10 @@ Deciding to move on... The dialog should: - [] Open and close correctly -- [] Avoid using any external UI libraries - we'd like to see your own implementation +- [x] Avoid using any external UI libraries - we'd like to see your own implementation - [] Be keyboard accessible - including Escape to close, and correct focus management when opening and closing -- [] Follow the existing component patterns, naming conventions, and file structure in the codebase -- [] Be written in TypeScript +- [x] Follow the existing component patterns, naming conventions, and file structure in the codebase +- [1/2] Be written in TypeScript << GOT STUCK HERE on the open closer functions >> - [] Mobile behaviour and breakpoints - [] I'd want to fix main background, so scroll only affects the content of the dialog when open diff --git a/src/Component/Dialog/Dialog.stories.ts b/src/Component/Dialog/Dialog.stories.ts index 5ec5aa5..4d06426 100644 --- a/src/Component/Dialog/Dialog.stories.ts +++ b/src/Component/Dialog/Dialog.stories.ts @@ -7,9 +7,8 @@ import { Heading as HeadingType, HeadingTypes, WysiwygText } from "@pnx-mixtape/ export type Dialog = { title?: HeadingType - toggleAll?: boolean content: WysiwygText - open?: boolean + state?: boolean id?: string } @@ -21,7 +20,6 @@ const meta: Meta = { title: "This is a HTML Dialog Element", as: HeadingTypes.TWO, }), - content: "This is the default story content text", }, argTypes: { title: { @@ -30,7 +28,7 @@ const meta: Meta = { control: "text", }, content: { - description: "Add text for the dialog", + description: "Add text for the dialog 1", control: "text", }, }, @@ -39,17 +37,16 @@ const meta: Meta = { export default meta type Story = StoryObj -export const Dialog: Story = {} - -export const DefaultOpen: Story = { +export const Dialog: Story = { args: { - ...meta.args, - content: "This is the default story content text", + content: "hi there", }, } -export const ToggleAll: Story = { +export const DefaultOpen: Story = { args: { - toggleAll: true, + ...meta.args, + state: true, + content: "This is the default story content text 2", }, } diff --git a/src/Component/Dialog/Elements/Dialog.ts b/src/Component/Dialog/Elements/Dialog.ts index 7e7bdd6..3d7be96 100644 --- a/src/Component/Dialog/Elements/Dialog.ts +++ b/src/Component/Dialog/Elements/Dialog.ts @@ -16,16 +16,16 @@ export default class Dialog extends HTMLElement { } connectedCallback(): void { - if (!this.dialog || !this.trigger) return + if (!this.dialogElement || !this.trigger) return const { signal }: AbortController = this.controller - document.addEventListener("beforeprint", this.handleOpen, { - signal, - }) - document.addEventListener("afterprint", this.handleClose, { + + document.addEventListener("click", this.handleToggle, { signal, }) + this.handleHash() + document.addEventListener("hashchange", this.handleHash, { signal }) } @@ -33,40 +33,58 @@ export default class Dialog extends HTMLElement { this.controller.abort() } - handleOpen = (): void => { - if (!this.dialog) return - this.dialog.open = true + handleToggle = (): void => { + if (!this.dialogElement) return + + // This was temporary - wanted to make a open close function. + if (this.dialogElement.dataset.state === "closed") { + this.dialogElement.setAttribute("data-state", "open") + } else { + this.dialogElement.setAttribute("data-state", "closed") + } } handleClose = (): void => { - if (!this.dialog) return - this.dialog.open = false + if (!this.closer) return } handleHash = (): void => { const { hash }: Location = window.location - if (hash && hash === `#${this.dialog?.id}`) { - this.handleOpen() + if (hash && hash === `#${this.dialogElement?.id}`) { + this.handleToggle() } } - get dialog(): HTMLDialogElement | null { - const dialog: HTMLDialogElement | null = this.querySelector("dialog") - if (!dialog) { - throw new Error(`${this.localName} must contain a element.`) + get dialogElement(): HTMLElement | null { + const dialogElement: HTMLElement | null = this.querySelector(".mx-dialog__element") + + if (!dialogElement) { + throw new Error(`${this.localName} must contain an element with .mx-dialog__element class.`) } - dialog.id = dialog.id || this.generatedId() - return dialog + dialogElement.id = dialogElement.id || this.generatedId() + return dialogElement } get trigger(): HTMLElement | null { - const trigger: HTMLElement | null = this.querySelector("summary") + const trigger: HTMLElement | null = this.querySelector(".mx-dialog__toggle") + if (!trigger) { - throw new Error(`${this.localName} must contain a element.`) + throw new Error(`${this.localName} must contain an element with class="mx-dialog__toggle">.`) } return trigger } + get closer(): HTMLElement | null { + const closer: HTMLElement | null = this.querySelector(".mx-dialog__element__close") + + if (!closer) { + throw new Error( + `${this.localName} must contain an element with class="mx-dialog__element__close">.`, + ) + } + return closer + } + generatedId = (): string => { const string: string | undefined = this.trigger?.textContent?.trim() return !string ? "" : makeAnchor(string) diff --git a/src/Component/Dialog/dialog.css b/src/Component/Dialog/dialog.css index 884820f..ec0f317 100644 --- a/src/Component/Dialog/dialog.css +++ b/src/Component/Dialog/dialog.css @@ -21,24 +21,27 @@ align-items: center; } - .mx-dialog { - & .mx-dialog__toggle { - cursor: pointer; - inline-size: 100%; - } + .mx-dialog__toggle { + cursor: pointer; + inline-size: auto; + appearance: none; + } - &:is(> h2, > h3, > h4, > h5, > h6) { + .mx-dialog__element { + & &:is(> h2, > h3, > h4, > h5, > h6) { all: unset; } - & > .mx-dialog__content { + > .mx-dialog__content { display: none; opacity: 0; overflow: hidden; } - &:is([open], [data-open="true"]) { - & > .mx-dialog__content { + &[data-state="open"] { + background: red; + + .mx-dialog__content { opacity: 1; display: block; diff --git a/src/Component/Dialog/dialog.twig b/src/Component/Dialog/dialog.twig index 44859a5..88e84f9 100644 --- a/src/Component/Dialog/dialog.twig +++ b/src/Component/Dialog/dialog.twig @@ -1,17 +1,22 @@ -{% set baseClass = 'mx-dialog' %} +{% set baseClass = 'mx-dialog__element' %} {% set classes = [ baseClass, ] %} {% set attributes = (attributes ?? create_attribute()).addClass(classes) %} - - {# uses HTML Dialog Element #} - + + - -

{{ title }}

- {{ content }} + +
+ {{ title }} + {{ content }} - -
+ + + + From 99092f7706dc247bcb80dd98652e211ce7d79b7f Mon Sep 17 00:00:00 2001 From: Stewest Date: Sat, 28 Mar 2026 16:01:42 +1300 Subject: [PATCH 3/9] basic styling - check open close : bug 4h30 --- src/Component/Dialog/Dialog.stories.ts | 23 ++++++++-- src/Component/Dialog/Elements/Dialog.ts | 6 ++- src/Component/Dialog/dialog.css | 59 ++++++++++++++++++------- src/Component/Dialog/dialog.twig | 33 +++++++++----- 4 files changed, 90 insertions(+), 31 deletions(-) diff --git a/src/Component/Dialog/Dialog.stories.ts b/src/Component/Dialog/Dialog.stories.ts index 4d06426..f3732e6 100644 --- a/src/Component/Dialog/Dialog.stories.ts +++ b/src/Component/Dialog/Dialog.stories.ts @@ -3,13 +3,17 @@ import Component from "./dialog.twig" import Heading from "../../Atom/Heading/heading.twig" import "./Elements/Dialog" import "./dialog.css" +import "../Card/card.css" import { Heading as HeadingType, HeadingTypes, WysiwygText } from "@pnx-mixtape/ids-shape" export type Dialog = { title?: HeadingType content: WysiwygText + dialogTitle: HeadingType + dialogContent: WysiwygText state?: boolean id?: string + toggleText?: string } const meta: Meta = { @@ -17,7 +21,11 @@ const meta: Meta = { component: Component, args: { title: Heading({ - title: "This is a HTML Dialog Element", + title: "Closed state 'Dialog card' element title", + as: HeadingTypes.TWO, + }), + dialogTitle: Heading({ + title: "This is the open Custom 'Dialog' Element title", as: HeadingTypes.TWO, }), }, @@ -28,6 +36,15 @@ const meta: Meta = { control: "text", }, content: { + description: "Add text for the initial Dialog card", + control: "text", + }, + dialogTitle: { + description: + "Optional [Heading](/?path=/docs/atom-heading--docs) component, displayed above the Dialog.", + control: "text", + }, + dialogContent: { description: "Add text for the dialog 1", control: "text", }, @@ -39,7 +56,8 @@ type Story = StoryObj export const Dialog: Story = { args: { - content: "hi there", + content: "This is the default story content text inside the dialog card part 1", + dialogContent: "This is the default story content text inside the dialog part 2", }, } @@ -47,6 +65,5 @@ export const DefaultOpen: Story = { args: { ...meta.args, state: true, - content: "This is the default story content text 2", }, } diff --git a/src/Component/Dialog/Elements/Dialog.ts b/src/Component/Dialog/Elements/Dialog.ts index 3d7be96..40f300e 100644 --- a/src/Component/Dialog/Elements/Dialog.ts +++ b/src/Component/Dialog/Elements/Dialog.ts @@ -16,7 +16,7 @@ export default class Dialog extends HTMLElement { } connectedCallback(): void { - if (!this.dialogElement || !this.trigger) return + if (!this.dialogElement || !this.trigger || !this.closer) return const { signal }: AbortController = this.controller @@ -24,6 +24,10 @@ export default class Dialog extends HTMLElement { signal, }) + document.addEventListener("click", this.handleClose, { + signal, + }) + this.handleHash() document.addEventListener("hashchange", this.handleHash, { signal }) diff --git a/src/Component/Dialog/dialog.css b/src/Component/Dialog/dialog.css index ec0f317..9a8944f 100644 --- a/src/Component/Dialog/dialog.css +++ b/src/Component/Dialog/dialog.css @@ -13,6 +13,18 @@ } @layer design-system.components { + /* start to look at making body not scroll when Dialog is open */ + body:has(.mx-dialog__card .mx-dialog__element[data-state="open"]) { + overflow-y: hidden; + } + .mx-dialog__card { + margin-block-end: var(--spacing-l); + article { + width: 100%; + padding: var(--spacing-l); + } + } + .mx-dialog__title { margin-block-end: var(--spacing-m); display: flex; @@ -21,34 +33,49 @@ align-items: center; } + .mx-dialog__element__close, .mx-dialog__toggle { cursor: pointer; inline-size: auto; - appearance: none; + justify-content: space-between; } .mx-dialog__element { - & &:is(> h2, > h3, > h4, > h5, > h6) { - all: unset; - } - - > .mx-dialog__content { - display: none; - opacity: 0; - overflow: hidden; - } + display: none; + opacity: 0; + overflow: hidden; + height: 1px; &[data-state="open"] { - background: red; + position: fixed; + opacity: 1; + display: block; + inset: 0; + width: 100%; + height: 100%; + background-color: rgba(0, 0, 0, 0.3); + z-index: 10; - .mx-dialog__content { + @starting-style { + opacity: 0; + display: block; + bottom: 0; + position: fixed; opacity: 1; display: block; + inset: 0; + width: 50%; + } + .mx-dialog__content { + --scheme: var(--alt-scheme, var(--colour-scheme)); + --background: var(--colour-background-alt); - @starting-style { - opacity: 0; - display: block; - } + background-color: var(--background); + margin: auto; + inset: 2rem; + position: absolute; + z-index: 20; + padding: 2rem; } } } diff --git a/src/Component/Dialog/dialog.twig b/src/Component/Dialog/dialog.twig index 88e84f9..7f5c738 100644 --- a/src/Component/Dialog/dialog.twig +++ b/src/Component/Dialog/dialog.twig @@ -4,19 +4,30 @@ ] %} {% set attributes = (attributes ?? create_attribute()).addClass(classes) %} - - - -
- {{ title }} - {{ content }} - - + +
+
+

{{title}}

+ +
+ {{content}}
+
+ + +
+
+ {{ dialogTitle }} + +
+
{{ dialogContent }}
+
From df193bc65a4536001d06fe407847f1e37ffa5f45 Mon Sep 17 00:00:00 2001 From: Stewest Date: Sat, 28 Mar 2026 17:04:22 +1300 Subject: [PATCH 4/9] finally understood the Dialog.ts events --- src/Component/Dialog/Dialog.stories.ts | 34 ++++++++++++++--- src/Component/Dialog/Elements/Dialog.ts | 50 +++++++++++++------------ src/Component/Dialog/dialog.css | 26 +++++++++---- src/Component/Dialog/dialog.twig | 12 +++--- 4 files changed, 81 insertions(+), 41 deletions(-) diff --git a/src/Component/Dialog/Dialog.stories.ts b/src/Component/Dialog/Dialog.stories.ts index f3732e6..00b4672 100644 --- a/src/Component/Dialog/Dialog.stories.ts +++ b/src/Component/Dialog/Dialog.stories.ts @@ -28,6 +28,11 @@ const meta: Meta = { title: "This is the open Custom 'Dialog' Element title", as: HeadingTypes.TWO, }), + content: + "

Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.

", + dialogContent: + "

Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.

", + state: false, }, argTypes: { title: { @@ -36,8 +41,11 @@ const meta: Meta = { control: "text", }, content: { - description: "Add text for the initial Dialog card", + description: "Content.", control: "text", + type: { + name: "string", + }, }, dialogTitle: { description: @@ -45,8 +53,18 @@ const meta: Meta = { control: "text", }, dialogContent: { - description: "Add text for the dialog 1", + description: "Content.", control: "text", + type: { + name: "string", + }, + }, + state: { + description: "Dialog open or closed", + control: "boolean", + table: { + defaultValue: { summary: "closed" }, + }, }, }, } @@ -56,14 +74,20 @@ type Story = StoryObj export const Dialog: Story = { args: { - content: "This is the default story content text inside the dialog card part 1", - dialogContent: "This is the default story content text inside the dialog part 2", + content: + "

This is the default story content text inside the dialog card part 1. Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.

", + dialogContent: + "

Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.

Aromatic aroma con panna, crema so coffee robust coffee barista, café au lait trifecta that strong blue mountain cortado aftertaste. Aroma extraction french press, skinny sweet, blue mountain cup roast barista, beans, extra cappuccino mug crema strong. Americano caffeine white, con panna saucer sit, con panna eu, carajillo aftertaste kopi-luwak, body aftertaste cup single origin café au lait saucer

Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.

", }, } -export const DefaultOpen: Story = { +/** + * Open by default dialog. + */ +export const StateOpen: Story = { args: { ...meta.args, state: true, + id: "open-dialog", }, } diff --git a/src/Component/Dialog/Elements/Dialog.ts b/src/Component/Dialog/Elements/Dialog.ts index 40f300e..4b1869f 100644 --- a/src/Component/Dialog/Elements/Dialog.ts +++ b/src/Component/Dialog/Elements/Dialog.ts @@ -16,16 +16,18 @@ export default class Dialog extends HTMLElement { } connectedCallback(): void { - if (!this.dialogElement || !this.trigger || !this.closer) return + if (!this.dialogElement || !this.openTrigger || !this.closerTigger) return const { signal }: AbortController = this.controller - document.addEventListener("click", this.handleToggle, { + document.addEventListener("click", this.handleClick, { signal, }) - document.addEventListener("click", this.handleClose, { - signal, + document.addEventListener("keydown", event => { + if (event.code === "Escape") { + this.handleClose() + } }) this.handleHash() @@ -37,25 +39,27 @@ export default class Dialog extends HTMLElement { this.controller.abort() } - handleToggle = (): void => { - if (!this.dialogElement) return - - // This was temporary - wanted to make a open close function. - if (this.dialogElement.dataset.state === "closed") { - this.dialogElement.setAttribute("data-state", "open") - } else { - this.dialogElement.setAttribute("data-state", "closed") + handleClick = ({ target }) => { + if (target === this.openTrigger) { + this.handleOpen() + } + if (target === this.closerTigger) { + this.handleClose() } } + handleOpen = (): void => { + this.dialogElement.setAttribute("data-state", "open") + } + handleClose = (): void => { - if (!this.closer) return + this.dialogElement.setAttribute("data-state", "closed") } handleHash = (): void => { const { hash }: Location = window.location if (hash && hash === `#${this.dialogElement?.id}`) { - this.handleToggle() + this.handleOpen() } } @@ -69,28 +73,28 @@ export default class Dialog extends HTMLElement { return dialogElement } - get trigger(): HTMLElement | null { - const trigger: HTMLElement | null = this.querySelector(".mx-dialog__toggle") + get openTrigger(): HTMLElement | null { + const trigger: HTMLElement | null = this.querySelector(".mx-dialog__toggle button") if (!trigger) { - throw new Error(`${this.localName} must contain an element with class="mx-dialog__toggle">.`) + throw new Error(`${this.localName} must contain an element with .mx-dialog__toggle>.`) } return trigger } - get closer(): HTMLElement | null { - const closer: HTMLElement | null = this.querySelector(".mx-dialog__element__close") + get closerTigger(): HTMLElement | null { + const trigger: HTMLElement | null = this.querySelector(".mx-dialog__element__close button") - if (!closer) { + if (!trigger) { throw new Error( - `${this.localName} must contain an element with class="mx-dialog__element__close">.`, + `${this.localName} must contain an element with class mx-dialog__element__close>.`, ) } - return closer + return trigger } generatedId = (): string => { - const string: string | undefined = this.trigger?.textContent?.trim() + const string: string | undefined = this.openTrigger?.textContent?.trim() return !string ? "" : makeAnchor(string) } } diff --git a/src/Component/Dialog/dialog.css b/src/Component/Dialog/dialog.css index 9a8944f..7f0a5d0 100644 --- a/src/Component/Dialog/dialog.css +++ b/src/Component/Dialog/dialog.css @@ -23,6 +23,20 @@ width: 100%; padding: var(--spacing-l); } + + button { + mask-size: var(--spacing-l); + appearance: none; + border: none; + background-color: transparent; + display: block; + } + + &:has(.mx-dialog__element[data-state="open"]) { + --line-colour: tranparent; + + border: 0; + } } .mx-dialog__title { @@ -53,18 +67,15 @@ inset: 0; width: 100%; height: 100%; - background-color: rgba(0, 0, 0, 0.3); + background-color: rgba(0, 0, 0, 0.7); z-index: 10; @starting-style { opacity: 0; display: block; bottom: 0; - position: fixed; - opacity: 1; - display: block; - inset: 0; - width: 50%; + width: 5%; + height: 1px; } .mx-dialog__content { --scheme: var(--alt-scheme, var(--colour-scheme)); @@ -76,6 +87,7 @@ position: absolute; z-index: 20; padding: 2rem; + overflow-y: auto; } } } @@ -87,7 +99,7 @@ @media print { .mx-dialog { - & .mx-dialog__content { + .mx-dialog__content { display: block !important; } } diff --git a/src/Component/Dialog/dialog.twig b/src/Component/Dialog/dialog.twig index 7f5c738..14de78e 100644 --- a/src/Component/Dialog/dialog.twig +++ b/src/Component/Dialog/dialog.twig @@ -4,27 +4,27 @@ ] %} {% set attributes = (attributes ?? create_attribute()).addClass(classes) %} +{% set dialogState = state ? "open" : "closed"%}

{{title}}

-
- {{content}} + {{content}}
- +
{{ dialogTitle }} -
{{ dialogContent }}
From ba4251fc21f3373f3c3d2fedbb9823d0d0cb4e92 Mon Sep 17 00:00:00 2001 From: Stewest Date: Sat, 28 Mar 2026 17:14:00 +1300 Subject: [PATCH 5/9] add snapshot --- .../__snapshots__/Dialog.stories.ts.snap | 69 +++++++++++++++++++ src/Component/Dialog/dialog.css | 29 ++++---- src/Component/Dialog/dialog.entry.js | 2 +- 3 files changed, 86 insertions(+), 14 deletions(-) create mode 100644 src/Component/Dialog/__snapshots__/Dialog.stories.ts.snap diff --git a/src/Component/Dialog/__snapshots__/Dialog.stories.ts.snap b/src/Component/Dialog/__snapshots__/Dialog.stories.ts.snap new file mode 100644 index 0000000..42ffbcb --- /dev/null +++ b/src/Component/Dialog/__snapshots__/Dialog.stories.ts.snap @@ -0,0 +1,69 @@ +// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html + +exports[`Dialog 1`] = ` +" + + +
+
+

+

Closed state 'Dialog card' element title

+ + +
+

This is the default story content text inside the dialog card part 1. Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.

+
+
+ + +
+
+
+ +

This is the open Custom 'Dialog' Element title

+ + +
+

Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.

Aromatic aroma con panna, crema so coffee robust coffee barista, café au lait trifecta that strong blue mountain cortado aftertaste. Aroma extraction french press, skinny sweet, blue mountain cup roast barista, beans, extra cappuccino mug crema strong. Americano caffeine white, con panna saucer sit, con panna eu, carajillo aftertaste kopi-luwak, body aftertaste cup single origin café au lait saucer

Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.

+
+
+ +" +`; + +exports[`State Open 1`] = ` +" + + +
+
+

+

Closed state 'Dialog card' element title

+ + +
+

Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.

+
+
+ + +
+
+
+ +

This is the open Custom 'Dialog' Element title

+ + +
+

Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.

+
+
+ +" +`; diff --git a/src/Component/Dialog/dialog.css b/src/Component/Dialog/dialog.css index 7f0a5d0..d960bb0 100644 --- a/src/Component/Dialog/dialog.css +++ b/src/Component/Dialog/dialog.css @@ -17,18 +17,20 @@ body:has(.mx-dialog__card .mx-dialog__element[data-state="open"]) { overflow-y: hidden; } + .mx-dialog__card { margin-block-end: var(--spacing-l); - article { - width: 100%; + + & article { + inline-size: 100%; padding: var(--spacing-l); } - button { + & button { mask-size: var(--spacing-l); appearance: none; border: none; - background-color: transparent; + background-color: var(--colour-primary-light); display: block; } @@ -58,26 +60,27 @@ display: none; opacity: 0; overflow: hidden; - height: 1px; + block-size: 1px; &[data-state="open"] { position: fixed; opacity: 1; display: block; inset: 0; - width: 100%; - height: 100%; - background-color: rgba(0, 0, 0, 0.7); + inline-size: 100%; + block-size: 100%; + background-color: rgb(0 0 0 / 70%); z-index: 10; @starting-style { opacity: 0; display: block; - bottom: 0; - width: 5%; - height: 1px; + inset-block-end: 0; + inline-size: 5%; + block-size: 1px; } - .mx-dialog__content { + + & .mx-dialog__content { --scheme: var(--alt-scheme, var(--colour-scheme)); --background: var(--colour-background-alt); @@ -99,7 +102,7 @@ @media print { .mx-dialog { - .mx-dialog__content { + & .mx-dialog__content { display: block !important; } } diff --git a/src/Component/Dialog/dialog.entry.js b/src/Component/Dialog/dialog.entry.js index e25246b..e4ae648 100644 --- a/src/Component/Dialog/dialog.entry.js +++ b/src/Component/Dialog/dialog.entry.js @@ -1 +1 @@ -import './Elements/Dialog' +import "./Elements/Dialog" From 35fca43f2db6f3b529d347dc89467b7c70268875 Mon Sep 17 00:00:00 2001 From: Stewest Date: Sat, 28 Mar 2026 17:17:43 +1300 Subject: [PATCH 6/9] (docs): update readme --- README.md | 34 +++++++++++++++++++--------------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index 4b8fe1b..7da3d0b 100644 --- a/README.md +++ b/README.md @@ -59,31 +59,34 @@ Deciding to move on... - [x] Iterate - [x] Test assumptions - [x] Create storybook component - - [] Style a Story. - - [] Add modifiers for Dialog storybook component + - [x] Style a Story. + - [x] Check Accessibility + - [x] Add modifiers for Dialog storybook component - [] Make a Story with multiple dialogs to test open / close of each on a page. - [] Style dialogs on a page and ::backdrop. - - [ ] Write tests + docs - - [ ] commits + - [ ] update snapshots & Write interaction tests + - [x] docs + - [x] commits ## Requirement checklist The dialog should: -- [] Open and close correctly +- [x] Open and close correctly - [x] Avoid using any external UI libraries - we'd like to see your own implementation -- [] Be keyboard accessible - including Escape to close, and correct focus management when opening and closing -- [x] Follow the existing component patterns, naming conventions, and file structure in the codebase -- [1/2] Be written in TypeScript << GOT STUCK HERE on the open closer functions >> +- [x] Be keyboard accessible - including Escape to close, +- [ ] And correct focus management when opening and closing +- [1/2] Follow the existing component patterns, naming conventions, and file structure in the codebase +- [x] Be written in TypeScript << GOT STUCK HERE on the open / closer functions >> - [] Mobile behaviour and breakpoints -- [] I'd want to fix main background, so scroll only affects the content of the dialog when open +- [x] I'd want to fix main background, so scroll only affects the content of the dialog when open - [] Animation and transition behaviour -- [] How the trigger element is handled +- [x] How the trigger element is handled -- [] Run tests, linters -- [] Docs -- [] Commit +- [x] Run tests, linters +- [x] Docs +- [x] Commit ## Submission @@ -95,9 +98,10 @@ The dialog should: - [x] But suggestion is to use TypeScript, so will do a more manual custom dialog - Any trade-offs or decisions you'd approach differently with more time - - [x] Anything you noticed in the existing codebase you'd flag in a code review - -- NPM packages security audit - 1 critical package + - The format/linter changes semi quotes back to double quotes + - I'd get more familiar with the available utility styles and colours and spacing css vars. + - NPM packages security audit - 1 critical package - [x] include a README (this): From 521248eb3137e51966fe7c4d9832faf3989348d7 Mon Sep 17 00:00:00 2001 From: Stewest Date: Sat, 28 Mar 2026 17:45:22 +1300 Subject: [PATCH 7/9] return elements to the way they were - but some of these are missing. --- src/elements.ts | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/elements.ts b/src/elements.ts index 4fd8688..664db52 100644 --- a/src/elements.ts +++ b/src/elements.ts @@ -1,13 +1,13 @@ export { default as Accordion } from "./Component/Accordion/Elements/Accordion" export { default as AccordionGroup } from "./Component/Accordion/Elements/AccordionGroup" -// export { default as AccordionMobile } from "./Component/Accordion/Elements/AccordionMobile" -// export { default as AccordionDiv } from "./Component/Accordion/Elements/AccordionDiv" +export { default as AccordionMobile } from "./Component/Accordion/Elements/AccordionMobile" +export { default as AccordionDiv } from "./Component/Accordion/Elements/AccordionDiv" export { default as Dialog } from "./Component/Dialog/Elements/Dialog" -// export { default as DropMenu } from "./Component/DropMenu/Elements/DropMenu" -// export { default as ClosableAlert } from "./Component/GlobalAlert/Elements/ClosableAlert" -// export { default as Sticky } from "./Component/Sticky/Elements/Sticky" -// export { default as Tabs } from "./Component/Tabs/Elements/Tabs" -// export { default as GlobalToggle } from "./Layout/Header/Elements/GlobalToggle" -// export { default as InPageNavigation } from "./Component/InPageNavigation/Elements/InPageNavigation" -// export { default as Navigation } from "./Component/Navigation/Elements/Navigation" +export { default as DropMenu } from "./Component/DropMenu/Elements/DropMenu" +export { default as ClosableAlert } from "./Component/GlobalAlert/Elements/ClosableAlert" +export { default as Sticky } from "./Component/Sticky/Elements/Sticky" +export { default as Tabs } from "./Component/Tabs/Elements/Tabs" +export { default as GlobalToggle } from "./Layout/Header/Elements/GlobalToggle" +export { default as InPageNavigation } from "./Component/InPageNavigation/Elements/InPageNavigation" +export { default as Navigation } from "./Component/Navigation/Elements/Navigation" export * from "./Utility/utilities" From f44f902ef4ad9b7392e19ea4353943ace5ad90a4 Mon Sep 17 00:00:00 2001 From: Stewest Date: Sat, 28 Mar 2026 17:45:46 +1300 Subject: [PATCH 8/9] remove handleHash - as thats uneeded --- src/Component/Dialog/Elements/Dialog.ts | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/src/Component/Dialog/Elements/Dialog.ts b/src/Component/Dialog/Elements/Dialog.ts index 4b1869f..c396948 100644 --- a/src/Component/Dialog/Elements/Dialog.ts +++ b/src/Component/Dialog/Elements/Dialog.ts @@ -29,10 +29,6 @@ export default class Dialog extends HTMLElement { this.handleClose() } }) - - this.handleHash() - - document.addEventListener("hashchange", this.handleHash, { signal }) } disconnectedCallback(): void { @@ -56,13 +52,6 @@ export default class Dialog extends HTMLElement { this.dialogElement.setAttribute("data-state", "closed") } - handleHash = (): void => { - const { hash }: Location = window.location - if (hash && hash === `#${this.dialogElement?.id}`) { - this.handleOpen() - } - } - get dialogElement(): HTMLElement | null { const dialogElement: HTMLElement | null = this.querySelector(".mx-dialog__element") From f5390b3ab01a8d9bafe3b325624fdfbce7666c06 Mon Sep 17 00:00:00 2001 From: Stewest Date: Sat, 28 Mar 2026 17:46:16 +1300 Subject: [PATCH 9/9] (Docs): Update trade offs and review thoughts --- README.md | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 7da3d0b..a3a8156 100644 --- a/README.md +++ b/README.md @@ -94,13 +94,17 @@ The dialog should: - [x] whether to use dialog element - https://caniuse.com/?search=dialog - https://developer.mozilla.org/en-US/docs/Web/HTML/Reference/Elements/dialog > Global 96.86% + - [] or latest popover with polyfills for browsers not yet compatible. - [x] But suggestion is to use TypeScript, so will do a more manual custom dialog -- Any trade-offs or decisions you'd approach differently with more time +- [x] Any trade-offs or decisions you'd approach differently with more time + - Get more familiar with the available utility styles and colours and spacing css vars. + - Read the Function docs! + - Investigate and Fix the other Failed tests + - [x] Anything you noticed in the existing codebase you'd flag in a code review - - The format/linter changes semi quotes back to double quotes - - I'd get more familiar with the available utility styles and colours and spacing css vars. + - The format/linter changes single quotes back to double quotes (but "semi": false, inoxfmtrc.json) - NPM packages security audit - 1 critical package - [x] include a README (this):