Vonage / vivid-3

Vonage's web UI 🎨 toolbelt
https://vivid.deno.dev
Apache License 2.0
51 stars 11 forks source link

[Dialog]: Adding separators #910

Closed AyalaBu closed 1 year ago

AyalaBu commented 1 year ago

There are a few layout designs for the dialog layout, one of them contains a separators - it appears here:

https://www.figma.com/file/JJNgZvt1qf3ydYmOwbE3Jg/Vivid-UI-Kit---3.0-WIP?node-id=19420%3A160156&t=mBYOHp9Om3VmCrkJ-3

I think that this feature is important, because it orders a content of the component.

related Jira ticket: https://jira.vonage.com/browse/VIV-790

rachelbt commented 1 year ago

first question that need to be answered is weather we are adding the divider or are the consumers doing it with adding vwc-divider.

either way we need to extract the inline-padding from main-wrapper class and have all 3 parts - header, content and footer. We need to bear in mind that this also means that we need to no-render these parts if there's no content to avoid having un-necessary spaces.

If we are implanting the divider - we need to decide if it is related to the header + footer, the content or regardless.

yinonov commented 1 year ago

what's the difference between the 2? when are dividers needed? when are they not? is there more than personal taste to it?

image
AyalaBu commented 1 year ago

@rachelbt @yinonov Although the divider has an aesthetic function as well, it also functions as an "order creator", since it divides the content to the easily understandable sections: header, content & footer

rachelbt commented 1 year ago

So there will be an option with divider and without?

AyalaBu commented 1 year ago

I rechecked with Joella's original design an this is the situation:

  1. Dialog with slotted content has separators
  2. Dialog without slotted content doesn't

So these are the variants that need to exist: Screenshot 2023-01-03 at 13 58 51

rachelbt commented 1 year ago

so we will either provide our users top / bottom / both divider member or let them add it themselves but either way we need to adjust the padding issue.

yinonov commented 1 year ago

I'd still ask, why should an author decide whether to add dividers or not. It can be enforced conditionally when requirements are met, as mentioned, when header, body and footer are present

AyalaBu commented 1 year ago

This is what I wrote: the author doesn't decide.

  1. Slotted content has dividers
  2. No slotted content - no dividers
rachelbt commented 1 year ago

I'd still ask, why should an author decide whether to add dividers or not. It can be enforced conditionally when requirements are met, as mentioned, when header, body and footer are present

or if there's only header and content...

yinonov commented 1 year ago

Ok. Sounds like a css work more than API

rachelbt commented 1 year ago

All of the examples on the right in Figma - that are suppose to reflect "no-content -> no divider" design are wrong. In the API we have header that contains icon-title-close button. The text beneath the is the content of the dialog. So basically it will need to have a divider too. image

Maybe we need to start with turning the text member to be sub-title (https://github.com/Vonage/vivid-3/issues/911).

yinonov commented 1 year ago

Yes. That's the convention

rachelbt commented 1 year ago

after discussing with design as follow up to this Jira ticket was decided that: