designsystemet icon indicating copy to clipboard operation
designsystemet copied to clipboard

Consistent components V1

Open mimarz opened this issue 1 year ago • 10 comments

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.

  • [ ] Investigate what naming we have used where and if it makes sense to do so.
  • [ ] Document discrepancies.
  • [ ] Decide on which components to rename.
### Tasks
- [ ] https://github.com/digdir/designsystemet/issues/2453

mimarz avatar Aug 12 '24 07:08 mimarz

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

Barsnes avatar Aug 12 '24 07:08 Barsnes

.Root

Komponenter som har .Root ved skrivande tid:

  • Accordion
  • Breadcrumbs
  • DropdownMenu
  • ErrorSummary
  • List
  • Modal
  • Pagination (med del-komponenter)
  • Popover
  • Tabs
  • ToggleGroup

Komponenter som har deler uten del-komponenter:

  • Card (wrapperen rundt alt)
    • Denne rendrer ein <div>, eller elementet du asChilder til. Denne er god kandidat for .Root
  • Table (wrapperen rundt alt)
    • Denne rendrer <table>, og kunne nok ha vært .Root, ellers hadde det blitt Table.Table

Andre ting som burde bli sett på

Generelle navn på props, navn og slikt.

Komponenter som kan ha navneendring

  • NativeSelect
    • Denne rendrer ein native <select>, foreslår Select som navn
  • Textfield
    • Denne rendrer ein native <input>, foreslår Input som navn
    • Me har ikkje endra navn på <textarea>
  • 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
  • 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
  • DropdownMenu.Content
    • Samme som over, foreslår .List

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

.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.

Barsnes avatar Aug 16 '24 11:08 Barsnes

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>

eirikbacker avatar Sep 10 '24 07:09 eirikbacker

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 avatar Sep 10 '24 11:09 mimarz

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 ha omfavnet .Context?

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

mimarz avatar Sep 10 '24 11:09 mimarz

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 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 avatar Sep 10 '24 11:09 Barsnes

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?

Barsnes avatar Sep 10 '24 12:09 Barsnes

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?

mimarz avatar Sep 10 '24 12:09 mimarz

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 🤔

Barsnes avatar Sep 10 '24 12:09 Barsnes

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

Ang. Select;

  • Støtter at NativeSelect bytter navn til Select - det er det vi rendrer, og vi kaller det heller ikke NativeTextarea. Har lest diskusjonen om Select og autocomplete og alt, men da er vi egentlig på vei til Combobox, eller input med tilhørende datalist, så Select er fint tenker jeg siden det er et etablert konsept 😊

Ang. Textfield;

  • Textfield rendrer en haug med saker ja, men det gjør også Textarea og Select.
    • I Gjensidige lanserte vi Input, Label, Select og Textarea, Datepicker, ValidationMessage og HelpText som individuelle komponenter, men de kunne alle legges som barn av FormField.
    • Ikke heelt 100% fornøyd med det APIet, fordi man egentlig kunne hatt FormField.Help og FormField.Error som compound components.
    • Men fordelen var at konsument slapp å sette opp Label for/Input id og aria-describedby - det gjorde FormField. Samtidig var APIet veldig 1:1 med HTML så det føltes kjent, så Input var en input.
    • Input og co kunne da brukes utenfor FormField men siden den da ikke fant noe context fra FormField, måtte man da sette opp id og slikt selv.
    • Det gav utviklere fleksibilitet, og ikke minst mulighet til å lage egne form-elementer som de bare kunne droppe inn i FormField for å få label/input wiring hjelpetekst og error-melding ut av boksen. For oss kunne FormField egentlig kanskje bare være en Field.Context, men er litt usikker på hvordan vi vil gjøre det med error-melding siden den egentlig må tegne hele tiden - også når den er skjult - for å leses i alle skjermlesere :o
    • Tenker uansett vi burde gjøre noe med APIet på dagens Select/Textfield/Textarea, da det er flere DOM-elementer under panseret som er skjult uten å brettes ut med compound component API - gjør det vanskelig å legge til egne attributter etc

Ang. Modal;

  • Tenker vi absolutt burde bare bruke Heading og vise eksempel heller enn å lage egne Modal.Heading. Også fordi subtitle= er et design som kanskje noen skal ha, men skal så mange ha en subtitle over hovedtittel? Hvordan setter jeg riktige levels? Kanskje jeg vil ha noe sånt på Card også? Bedre å la brukerne våre bruke byggeklossene til å sette sammen slike ting selv, og heller vise noen eksempler på mønster vi anbefaler i Storybook og Figma? 😊
  • Close-button kunne kanskje blitt en egen Modal.Close - litt usikker på hvor mye folk skal style lukk etc i forhold til å hvor mye vi skal eksponere den, men det kan vi jo se litt på når vi kommer til å jobbe med Modal? 😊

Ang. komponenter som krever Context;

  • Støttes med at vi kaster error - det er ganske vanlig praksis ☺️

Ang. komponenter som krever Accordion.Heading;

  • Tenker fortsatt vi skal vurdere om det heter Accordion eller Expandable eller Collapse ref #😊 ?
  • -Er det heading i det heletatt? Dersom vi skal legge oss på å benytte native details/summary (som jeg tror er lurt på sikt), så er det ikke noe heading-level, mao kanskje forvirrende å bruke ordet Heading. Skulle det vært Expandable.Summary eller Expandable.Button eller Expandable.Trigger (tenker kanskje .Summary siden vi begynner å etablere at .Trigger er noe som ligger før en komponent og .Button kan fort forvirres med at det brukes Button under panseret, mens .Summary er faktisk inni komponenten?)
<Expandable>
  <Expandable.Summary>What is your biggest wish?</Expandable.Summary>
  <Expandable.Content>Unlimited wishes</Expandable.Content>
</Expandable>

eirikbacker avatar Sep 10 '24 15:09 eirikbacker

We have done a lot of changes to component names, sub-components and move some options to hooks. Further work will be done in new issues under the same milestone

mimarz avatar Nov 26 '24 09:11 mimarz