`DialogPrimitive` shared components (HDS-3081)

[KristinLBradley]

---

The release of these changes is being put off to a future date. See related doc for more context around this decision.

:pushpin: Summary

If merged, this PR separates out shared components between Flyout & Modal as new "DialogPrimitive" components and uses them within the Modal & Flyout components.

• DialogPrimitive components web docs
• DialogPrimitive components showcase

Compare to:

• Flyout showcase
• Modal showcase

~~See also documentation branch: https://github.com/hashicorp/design-system/pull/1945~~ - merged in

:hammer_and_wrench: Detailed description

New DialogPrimitive components:

• Wrapper (layout container)
• Header
• Description (used only in Flyout)
• Body
• Footer

:link: External links

Jira ticket: HDS-3081

---

👀 Component checklist

• [x] Percy was checked for any visual regression
• [x] A changelog entry was added via Changesets if needed (see templates here)

:speech_balloon: Please consider using conventional comments when reviewing this PR.

[vercel[bot]]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated (UTC)
hds-showcase	✅ Ready (Inspect)	Visit Preview	Feb 22, 2024 0:22am
hds-website	✅ Ready (Inspect)	Visit Preview	Feb 22, 2024 0:22am

[KristinLBradley]

~~There are a couple of failing tests I still need to fix.~~ Could use feedback on things implemented so far.

[KristinLBradley]

@didoo

So currently I've created 4 new "Dialog Primitive" components:

• DialogPrimitive:Header
• DialogPrimitive:Description
• DialogPrimitive:Body
• DialogPrimitive:Footer

I updated the existing Modal and Flyout components to use these new primitive components.

What makes sense to me to do next is to create a new type of component to satisfy this consumer request: https://docs.google.com/document/d/1qJ9YKVwfyVz5pi-ooIv3cC-AZp1GANJL7rIOJN9IIBA/edit

I would call this new component Split Window Dialog. It would be basically like the Flyout but would be docked within a right-side slot added to the App Frame instead of overlaying other content.

This is what makes the most sense to me and what I think consumers would find most useful.

However, from what I think you (Cristiano) were saying, you don't think we should create a new Split Window Dialog component which is like the Modal and Flyout components?

What would we do instead though? Create a shared Layout (or "Wrapper") primitive component to contain the other primitives? IE: DialogPrimitive:Layout or something else?

[didoo]

For context, re. the conversation started by @KristinLBradley above, we're continuing the exploration directly in code and we'll report here once we have come to a conclusion/decision.

[extraclass]

You sent me a dozen emails and I have tried to unsubscribe with no luck. Please stop.

I have no idea what you are talking about! Please stop!

On Thu, Feb 8, 2024 at 11:10 PM Kristin Bradley @.***> wrote:

@.**** commented on this pull request.

In showcase/tests/integration/components/hds/dialog-primitive/header-test.js https://github.com/hashicorp/design-system/pull/1918#discussion_r1483865713 :

• test('it renders the title with icon and tagline if provided', async function (assert) {

•  await render(
  

•    hbs`
  

•    <Hds::DialogPrimitive::Header @icon="info" @tagline="Tagline">
  

•      Title
  

•    </Hds::DialogPrimitive::Header>
  

•  `
  

•  );
  

•  assert.dom('.hds-dialog-primitive__title').exists();
  

•  assert.dom('.hds-dialog-primitive__title').hasText('Tagline Title');
  

•  assert.dom('.hds-dialog-primitive__icon.flight-icon-info').exists();
  

•  assert.dom('.hds-dialog-primitive__tagline').exists();
  

•  assert.dom('.hds-dialog-primitive__tagline').hasText('Tagline');
  

• });
• // DISMISS

I focused on CSS updates to start will come back to tests tomorrow.

— Reply to this email directly, view it on GitHub https://github.com/hashicorp/design-system/pull/1918#discussion_r1483865713, or unsubscribe https://github.com/notifications/unsubscribe-auth/ALJ6D6K6UBXBPW2LFC5NBA3YSWVUVAVCNFSM6AAAAABCUA425WVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTQNZRGY3DGNBXGI . You are receiving this because you were mentioned.Message ID: @.***>

[didoo]

For future context: we decided to re-organize our "dialog-related" components following this new structure:

• Dialog
  • Dialog (the primitive)
  • Modal
  • Flyout

But after doing all the refactoring, we have realized that unfortunately the CSS class names (and component names) of the Modal and Flyout were heavily used across the consumers codebases (see here and here)

See also Slack thread here and here.

[didoo]

The Flyout header is now var(--token-color-surface-faint) where previously was inheriting var(--token-color-surface-primary) from the root element (left is before, right is after).

Yes, this is intentional.

Icons and Title in Modal header do not inherit the color anymore (as the Tagline does).

No, this was a regression. Fixed.

From an API perspective, in other instances across the library the @extraClass argument is named @contextualClass – should we try and align them or do you think they differ in scope?

Not sure why I thought it was called @extraClass. In any case, fixed.

P.S. The long content modal example is a great addition! 👏

Yeah, we missed that use case (and it bit me)

[hashicorp-cla]

All committers have signed the CLA.

[majedelass]

I added images and created a new sub folder within website/public/assets for dialog-primitives for shared assets. Wanted to confirm that everyone was ok with that?

[didoo]

I added images and created a new sub folder within website/public/assets for dialog-primitives for shared assets. Wanted to confirm that everyone was ok with that?

@majedelass not a blocker, but a consideration: have we maybe missed an opportunity here, while creating the images to replace the old sub-component instances, to have the component parts' representations more similar to the actual component (eg border radius and shadow, and a specific width) with blurred edges? See for example how it's done for the Table documentation, focusing on specific parts of the whole: https://helios.hashicorp.design/components/table

[majedelass]

Is that a style we are trying to standardize for documentation? If it is, then maybe we should go down that path.

[didoo]

Is that a style we are trying to standardize for documentation? If it is, then maybe we should go down that path.

@majedelass maybe sync with @jorytindall on this (he did a lot of these illustrations!)

[didoo]

For context: I have moved a commit that @majedelass made to replace the header/body/footer component instances with static images (context) to the documentation branch (https://github.com/hashicorp/design-system/pull/1945/commits/f100a1a657e0abc3400dad3801aaddc9589296c2) to avoid any confusion of where the documentation changes should go (this was a special case, so having this specific fix in this branch was OK too).

[KristinLBradley]

Nothing major, but one question for my own sanity, should some of the text elements be using more semantic elements rather than divs? Does it matter? I only ask because there are couple of different methods:

• The Description uses a div with the typography class helpers
• The Title and Tagline use the Text component with a div tag defined.

If that isn't a problem or has already been considered, please ignore this, just something that stood out to me.

@jorytindall I prefer using semantic elements wherever possible/practical but for these components I was just taking what had already been implemented for the Modal & Flyout and moved it to shared components so I didn't change any of the DOM really.

[didoo]

Nothing major, but one question for my own sanity, should some of the text elements be using more semantic elements rather than divs? Does it matter? I only ask because there are couple of different methods:

• The Description uses a div with the typography class helpers
• The Title and Tagline use the Text component with a div tag defined.

If that isn't a problem or has already been considered, please ignore this, just something that stood out to me.

@jorytindall I prefer using semantic elements wherever possible/practical but for these components I was just taking what had already been implemented for the Modal & Flyout and moved it to shared components so I didn't change any of the DOM really.

@jorytindall @KristinLBradley unless we use a <p> element for all of them (title, tagline and description), we can't use headings, because which heading level is the "right" one, to respect the order in the page, since we don't know (being primitives) where they will be injected/consumed, what is their context? or were you thinking of other HTML semantic tags?

[KristinLBradley]

Nothing major, but one question for my own sanity, should some of the text elements be using more semantic elements rather than divs? Does it matter? I only ask because there are couple of different methods:

• The Description uses a div with the typography class helpers
• The Title and Tagline use the Text component with a div tag defined.

If that isn't a problem or has already been considered, please ignore this, just something that stood out to me.

@jorytindall I prefer using semantic elements wherever possible/practical but for these components I was just taking what had already been implemented for the Modal & Flyout and moved it to shared components so I didn't change any of the DOM really.

@jorytindall @KristinLBradley unless we use a <p> element for all of them (title, tagline and description), we can't use headings, because which heading level is the "right" one, to respect the order in the page, since we don't know (being primitives) where they will be injected/consumed, what is their context? or were you thinking of other HTML semantic tags?

Yes, there would be an issue using heading tags due to hierarchy. I do prefer to use p tags to mark up pieces of text vs. divs but I don't think it would add much here so don't think there's a strong enough reason to change the markup from what it was before.

[didoo]

Now that we have #2213 we can close this one