search

digdir / designsystemet

Designsystemet
https://designsystemet.no
MIT License
82 stars 38 forks source link

Consistent components V1 #2221

Open mimarz opened 3 months ago

mimarz commented 3 months ago

Make sure we have a consistent naming for sub-components where a list of components is expected.

For example in Tabs we use .List for a list of children while in Pagination we use Content for a list for children.

Barsnes commented 3 months ago

Tab.List corresponds to its role="tablist", which makes sense to me. Pagination.Content should probably be .List since in renders an ul

Barsnes commented 3 months ago

.Root

Komponenter som har .Root ved skrivande tid:

Komponenter som har deler uten del-komponenter:

Andre ting som burde bli sett på

Generelle navn på props, navn og slikt.

Komponenter som kan ha navneendring

Komponenter med feil navn på andre ting

.Trigger

Dette er for komponenta med ein del som utføre ein trigger funksjon. Eksempler er Modal og Popover Denne navngivingen har me brukt konsistent på tvers, og eg finner ingen feil.

eirikbacker commented 2 months ago

From discussion 10.09.24 - suggest moving away from .Root convention and purifying to a explicit .Context and single-word-named component as base component:

We suggest using Component.Context if there is a Context not providing additional elements. We suggest using Component.Trigger if there is a optional wire-up element to create a trigger element. We suggest using Component as the "base" element.

<Card>
  Content
</Card>

<ToggleGroup>
  <ToggleGroup.List>
    <ToggleGroup.Item>Item 1</ToggleGroup.Item>
    <ToggleGroup.Item>Item 2</ToggleGroup.Item>
    <ToggleGroup.Item>Item 3</ToggleGroup.Item>
  </ToggleGroup.List>
</ToggleGroup>

<Pagination>
  <Pagination.List>
    <Pagination.Item>Item 1</Pagination.Item>
    <Pagination.Item>Item 2</Pagination.Item>
    <Pagination.Item>Item 3</Pagination.Item>
  </Pagination.List>
</Pagination>

<Breadcrumbs aria-label="Du er her:">
  <Breadcrumbs.List>
    <Breadcrumbs.Item>Item 1</Breadcrumbs.Item>
    <Breadcrumbs.Item>Item 2</Breadcrumbs.Item>
    <Breadcrumbs.Item>Item 3</Breadcrumbs.Item>
  </Breadcrumbs.List>
</Breadcrumbs>

<Modal.Context>
  <Modal.Trigger>Knapp</Modal.Trigger>
  <Modal>Content</Modal>
</Modal.Context>

<Tooltip.Context>
  <Tooltip.Trigger>Knapp</Tooltip.Trigger>
  <Tooltip>Content</Tooltip>
</Tooltip.Context>

<Dropdown.Context>
  <Dropdown.Trigger>Knapp</Dropdown.Trigger>
  <Dropdown>Content</Dropdown>
</Dropdown.Context>

<Popover.Context>
  <Popover.Trigger>Knapp</Popover.Trigger>
  <Popover>Content</Popover>
</Popover.Context>

<Button popovertarget="my-popover">Knapp</Button>
<Popover id="my-popover">Content</Popover>

<Table>
  <Table.Header>
    <Table.Row>
      <Table.HeaderCell>Header 1</Table.HeaderCell>
      <Table.HeaderCell>Header 2</Table.HeaderCell>
    </Table.Row>
  </Table.Header>  
  <Table.Body>
    <Table.Row>
      <Table.Cell>Header 1</Table.Cell>
      <Table.Cell>Header 2</Table.Cell>
    </Table.Row>
  </Table.Body>
</Table>
mimarz commented 2 months ago

Komponenter som kan ha navneendring

  • NativeSelect

    • Denne rendrer ein native <select>, foreslår Select som navn

Sånnsett enig, men dette må tas opp med resten av teamet. Sist vi snakket om dette så var det gode argumenter fra @Febakke og @mrosvik om å beholde "native" prefiksen.

  • Textfield

    • Denne rendrer ein native <input>, foreslår Input som navn

Textfield er brukt som en fellesbetegnelse for et textfelt som består av flere elementer. Vi har idag i tillegg til <input>; label, description, errormessage, character counter og prefix/suffix i denne komponenten. Ved er behov så er det heller en ny komponent som er kun en ren Input. Dette kan være mer aktuelt om vi ønsker å gå for en FormField tilnærming #2311

  • DropdownMenu

    • Dette er ein dropdown, og ikkje nødvendigvis ein meny. Burde skrive retningslinjer om at det kun skal være meny, eller omdøype til det den er

Ja vi bør se generelt nærmere på denne komponenten, om det skal være en vanlig meny eller overflow meny.

  • PaginationContent

    • Denne rendrer ut <ul>. På List har me brukt .Unordered og Ordered, men dette passa ikkje på Pagination. Eg foreslår .List, då denne harmonera fint med Tabs.List

Enig. Vi har vel bestemt oss for å gjøre denne endringen allerede i #2395

  • DropdownMenu.Content

    • Samme som over, foreslår .List

Enig

Komponenter med feil navn på andre ting

  • Accordion.Heading

    • Denne skifta frå .Header, men props og eksempel har ikkje blitt oppdatert likt

    • Kvifor har Modal og Card .Header?

    • Desse rendrer ikkje vår Heading komponent, og Accordion har skifta bort frå Header for å bedre vise det som blir rendra ut

Ja, det er noe forvirring i navngivning her, kanskje det gikk i ball når vi drev å endret på dette i etterkant. Modal.Header bruker faktisk vår Heading komponent så 🤷‍♂️. Ideelt så hadde det vært fint om vi kunne slippet disse ekstra sub komponenter som egentlig er en vanlig Heading. Må undersøkes nærmere

.Trigger

Dette er for komponenta med ein del som utføre ein trigger funksjon. Eksempler er Modal og Popover Denne navngivingen har me brukt konsistent på tvers, og eg finner ingen feil.

Jepp, her ble vi enige om å tilby begge tilnærminger?

mimarz commented 2 months ago

From discussion 10.09.24 - suggest moving away from .Root convention and purifying to a explicit .Context and single-word-named component as base component:

Vi må og bestemme oss for hva som skjer dersom du bruker en komponent som må ha omfavnet .Context?

Barsnes commented 2 months ago

From discussion 10.09.24 - suggest moving away from .Root convention and purifying to a explicit .Context and single-word-named component as base component:

Vi må og bestemme oss for hva som skjer dersom du bruker en komponent som må ha omfavnet .Context?

  • Fallbacks?
  • Silent fail?
  • Throw console error?

Funker det ikkje at react kaster error når den ikkje finner context den trenger? Eller vil du ha ein meir leselig error melding? Eg tenker at om ein komponent må ha contexten sin, så burde ting kræsje, altså ikkje funke.

Barsnes commented 2 months ago

Ja, det er noe forvirring i navngivning her, kanskje det gikk i ball når vi drev å endret på dette i etterkant. Modal.Header bruker faktisk vår Heading komponent så 🤷‍♂️. Ideelt så hadde det vært fint om vi kunne slippet disse ekstra sub komponenter som egentlig er en vanlig Heading. Må undersøkes nærmere

Nja, Modal.Header rendrer meir enn kun ein Heading, men det er det du sender inn som barn som havner i headingen 🫠 Kanskje me kan lage ein tilnærming om at når me bruker *.Header så er det forventa at konsument ordner med å legge inn Heading element sjølv?

mimarz commented 2 months ago

Ja, det er noe forvirring i navngivning her, kanskje det gikk i ball når vi drev å endret på dette i etterkant. Modal.Header bruker faktisk vår Heading komponent så 🤷‍♂️. Ideelt så hadde det vært fint om vi kunne slippet disse ekstra sub komponenter som egentlig er en vanlig Heading. Må undersøkes nærmere

Nja, Modal.Header rendrer meir enn kun ein Heading, men det er det du sender inn som barn som havner i headingen 🫠 Kanskje me kan lage ein tilnærming om at når me bruker *.Header så er det forventa at konsument ordner med å legge inn Heading element sjølv?

Trodde vi ble enige om å gjøre det slikt etter #2127, men glemte kanskje Modalen da, men ja, høres sånnset fornuftig ut, men har vi ikke noen bedre måte å løse lukke knappen på i Modal? Det @eirikbacker foreslo, eller om vi har en egen sub-component for close button?

Barsnes commented 2 months ago

Ja, det er noe forvirring i navngivning her, kanskje det gikk i ball når vi drev å endret på dette i etterkant. Modal.Header bruker faktisk vår Heading komponent så 🤷‍♂️. Ideelt så hadde det vært fint om vi kunne slippet disse ekstra sub komponenter som egentlig er en vanlig Heading. Må undersøkes nærmere

Nja, Modal.Header rendrer meir enn kun ein Heading, men det er det du sender inn som barn som havner i headingen 🫠 Kanskje me kan lage ein tilnærming om at når me bruker *.Header så er det forventa at konsument ordner med å legge inn Heading element sjølv?

Trodde vi ble enige om å gjøre det slikt etter #2127, men glemte kanskje Modalen da, men ja, høres sånnset fornuftig ut, men har vi ikke noen bedre måte å løse lukke knappen på i Modal? Det @eirikbacker foreslo, eller om vi har en egen sub-component for close button?

Om det Eirik foreslo funka, så tenker eg at det er beste løysingen :) men modalen skal ha både tittel og undertittel. Så undertittelen må då enten få sin eigen komponent, som heller ikkje ligger i Heading 🤔

eirikbacker commented 2 months ago

Masse nytt :) Hiver meg på i litt listeform jeg og;

Ang. Select;

Ang. Textfield;

Ang. Modal;

Ang. komponenter som krever Context;

Ang. komponenter som krever Accordion.Heading;