feat(dialog): added new component using base <dialog>

[agliga]

• fixes dialog: add support for native HTML dialog #435

Description

• Added <dialog> component. This is the skin/evo-web piece.
• Used a lot of the language from lightbox dialog and updated it to support dialog html element
• No more need of aria-modal="true" or role=dialog
• Added JS mainly for supporting the animation as well as the open/close for the buttons. This is in a separate file (dialog.ts).

Note

• There is a marko component but that is only there so the code does not get lost. Its not a working solution. That shouldn't affect anything since its on the evo-marko library

Screenshots

Large

Small

The only change to be before is that the small screen was positioned at the bottom. This is positioned in the middle as design prefers it this way.

[changeset-bot]

🦋 Changeset detected

Latest commit: 5fee7ac

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@ebay/skin	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[Copilot]

Pull request overview

Adds initial WIP documentation pages and Skin styles for a new Dialog component based on the native <dialog> element.

Changes:

• Added new Dialog docs pages (overview, CSS demo, accessibility guidelines) and metadata.
• Registered dialog in component metadata and included Skin dialog styles in the headless bundle.
• Introduced initial dialog.scss styling and open/close animation behavior.

Reviewed changes

Copilot reviewed 10 out of 12 changed files in this pull request and generated 13 comments.

Show a summary per file

File	Description
src/routes/_index/components/lightbox-dialog/+page.marko	Renames the wrapper id (likely to align with new dialog docs).
src/routes/_index/components/dialog/css+page.marko	Adds a CSS demo page with interactive native <dialog> behavior.
src/routes/_index/components/dialog/css+meta.json	Adds page metadata for the dialog CSS demo.
src/routes/_index/components/dialog/accessibility+page.marko	Adds dialog accessibility guidance content.
src/routes/_index/components/dialog/accessibility+meta.json	Adds page metadata for accessibility guidance.
src/routes/_index/components/dialog/+page.marko	Adds dialog overview/terminology documentation page.
src/routes/_index/components/dialog/+meta.json	Adds page metadata for the dialog docs landing page.
src/data/component-metadata.json	Registers the new dialog component and links to DS + storybook entries.
packages/skin/src/sass/dialog/dialog.scss	Adds base dialog styles and open/close animations.
packages/skin/src/sass/bundles/skin-headless.scss	Includes the dialog bundle in headless Skin build.

[ianmcburnie]

One thing we might want to lightly cover on this page is the new set of html attributes that work in tandem with dialog (e.g. Invoker Commands API). Unfortunately most of them we cannot use due to limited browser availability, but we might want to explicitly spell this out.

[agliga]

Does this work?
I pushed it as part of the review comments

  <h2>HTML Dialog Attributes</h2>
  <p>
    Modern browsers support the native <code>&lt;dialog&gt;</code> element along with 
    related HTML attributes such as the Invoker Commands API (e.g., <code>commandfor</code>, 
    <code>command</code>). While these attributes provide enhanced functionality for dialog 
    management, <strong>most are not yet widely supported across all browsers</strong>. 
    Due to limited browser availability, we cannot rely on these newer attributes in our 
    components at this time. We continue to use JavaScript-based approaches to ensure 
    consistent behavior across all supported browsers.
  </p>

[ianmcburnie]

Will we have similar JS logic in the Marko component? If so this would be a good candidate to put in our new js utilities (i.e. makeup)

[agliga]

I think that is scope creep for what this PR is about.
My understanding is that we shouldn't have any JS logic, except for the basics covered here (which is what we did here)

The logic might be similar but there would be extra boilerplate which can't be handled by basic javascript.

[ianmcburnie]

My understanding is that we shouldn't have any JS logic

If we identify any resuable logic, it should be pushed to a js utility module in the spirit of what we did with makeupjs. If there is resuable logic, and you want to do that in a follow up PR, or just create a ticket, that's fine, but it's good to be thinking of the bigger picture. I want to keep the spirit of makeupjs alive in Evo Web.

[ianmcburnie]

General comment: it would be nice if we could get back in the habit of providing screenshots in the description.

[ianmcburnie]

I'd kind of like to see a new vanilla js module/class for this dialog. It can handle the closing animation and closedby logic. e.g. @import 'dialog' from @evo-web/js/ui. That will be an easy way to see the gaps we still have to makeup based on browser policy, and provide a single source of truth for those.

[agliga]

General comment: it would be nice if we could get back in the habit of providing screenshots in the description.

I think its generally fine for new components (and if you look at my PRs I generally have them), but for this one they looked so just like the old one, it didn't seem necessary.
Plus the assumption is that people try it out locally.

I'd kind of like to see a new vanilla js module/class for this dialog. It can handle the closing animation and closedby logic. e.g. @import 'dialog' from @evo-web/js/ui. That will be an easy way to see the gaps we still have to makeup based on browser policy, and provide a single source of truth for those.

As I mentioned earlier, this seems like scope creep to what this PR should be about.

[agliga]

It should be ready for re-review for now.
I updated the text and such for now.

[ianmcburnie]

As I mentioned earlier, this seems like scope creep to what this PR should be about.

It's a shame we are considering this aspect of building a new component from the ground up as scope creep, I had hoped with the move to Evo Web we would carry on in the tradition of MIND patterns and makeup, but I can take care of it.

EDIT: So we talked offline and we agreed that there should be somewhere, if not in code, to describe the key parts of the behaviour that need to be covered. For dialog this would be the closedby logic and the animation logic. We can put this on a JavaScript tab (after the CSS tab). This acts as a spec for the framework implementations. If we don't do this, the danger is that the framework version implementations drift and we don't know which one is the source of truth.

[ianmcburnie]

I think its generally fine for new components (and if you look at my PRs I generally have them), but for this one they looked so just like the old one, it didn't seem necessary. Plus the assumption is that people try it out locally.

I would definitely appreciate screenshots. Some before and after screenshots are always useful. It doesn't need to be exhaustive.

[agliga]

So based on internal discussion, I added a JavaScript documentation page

For making the CSS page static there seems to be some issues with showing the dialogs inline (they show fine on large and small screens, but on medium screens, it overlaps the nav)
Will update with screenshots and we can take a call on that

[agliga]

Screenshots for inline dialogs

Again, these dialogs are anchored to be centered on the page and overlap all elements (including the container they are in. I set a height of 400px just for this version, otherwise the container was min height and it would overlap the demo examples)

Large and small look good. Medium has an overlap with the left nav.
I did not include it for this reason but if everyone else is happy with this I can.
Again, these are no-js versions of the dialogs, and non modal (you cannot open a modal dialog without js)

Large

Small

Medium (had overlap)

[LuLaValva]

Doesn't work in dark mode! Looks like <dialog> overrides color from its parent

Otherwise it looks good I think. The only potential thought is— do we want to keep this dialog--close behavior even though in theory we should be able to move away from it pretty soon and leave close animations as a progressive enhancement for newer browsers?

[agliga]

Doesn't work in dark mode! Looks like <dialog> overrides color from its parent

Otherwise it looks good I think. The only potential thought is— do we want to keep this `dialog--close` behavior even though _in theory_ we should be able to move away from it pretty soon and leave close animations as a progressive enhancement for newer browsers?

I believe we need to be on Safari 17 which will be a while.
I remember when we tested this there were some issues with the animation. If we can get it to all work on all browsers but maybe Safari <17 doesn't have a close animation, then we should be good.

[agliga]

Fixed dark mode by adding a color