Extend Md components' props with appropriate standard HTML attributes

[kajsaeggum]

Describe your changes

Justifying use case: Need to be able to set all different kinds of aria-props on MdInput.
aria-haspopup, aria-expanded, aria-autocomplete, aria-controls among others...

This PR extends the props of most components with the appropriate standard html attributes for the main element used within.
It also removes the use of [otherProps: string]: unknown, which previously allowed setting any prop on icons.
It also removes the use of ariaLabel as a prop, in favor of the built in aria-label prop.

This is set as a new major version because in some cases our previous custom props conflicted with one of the standard attributes and had to be renamed or got a change in type.

Breaking changes:

• size prop replaced with mode prop to control width of input component. Change affects: MdInput, MdAutocomplete, MdSelect, MdMultiSelect, MdTileVertical
• id prop can now only be of type string. Change affects: MdFilterChip, MdInputChip, MdCheckbox, MdRadioButton, MdAccordion, MdCheckboxGroup, MdMultiSelect, MdRadioGroup, MdSelect
• onChange prop replaced with onSelectOption as callback when an option is selected. Change affects: MdAutocomplete
• content prop replaced with tooltipContent to set the contents of the tooltip content. Change affects: MdTooltip
• ariaLabel prop replaced with standard aria-label prop. Change affects: MdIconButton, MdTooltip, MdHelpButton

Checklist before requesting a review

• I have performed a self-review and test of my code
• I have added label to the PR (major, minor or patch)
• If new component: Is story for component created in stories-folder?
• If new component: Is tsx-file import added to packages/react/index.tsx?
• If new component: Is css-file added to packages/css/index.css?

[github-actions]

Please set a versioning label of either major, minor, or patch to the pull request.

[kajsaeggum]

@aurorascharff @thomaslarsson Har dere noen tanker eller ev. andre forslag som unngår breaking changes?

[aurorascharff]

@aurorascharff @thomaslarsson Har dere noen tanker eller ev. andre forslag som unngår breaking changes?

Egentlig ikke. Dette er jo desperat nødvendig, haha. Har tenkt på det. selv. Edit: extend både gamle md props og inputprops, så fjern Md props extension senere. Går det an å depracate size og legge til både size og componentsize? Edit: det funker vel ikke likevel siden size tilhører inputprops. Så kanskje umulig å ikke ha breaking. Skal tenke litt mer.

[aurorascharff]

@aurorascharff @thomaslarsson Har dere noen tanker eller ev. andre forslag som unngår breaking changes?

Hva hvis

@aurorascharff @thomaslarsson Har dere noen tanker eller ev. andre forslag som unngår breaking changes?

Vi kan lage en ny mappe kalt unstable/formElements/MdInput, så bruker man denne nye og vi deprecater den gamle. Da vil kun importen endres. Vi burde gjøre det samme for MdTextArea i unstable/formElements/MdTextArea som også burde extende TextArea props. Kan du sjekke om det er flere elementer det gjelder? Så flytter vi tilbake og erstatter gamle versjon og fjerner unstable mappen igjen etterhvert. Virker som best practise er depracted -> obsolete neste versjon -> så fjerne den helt i en major.
Eventuelt så kan vi droppe dette da men da må vi si ifra til alle selvsagt. Men tror et fremdeles burde være en major men da også gjøre det på flere komponenter som har samme problem (textarea).

[kajsaeggum]

@aurorascharff @thomaslarsson Har dere noen tanker eller ev. andre forslag som unngår breaking changes?

Hva hvis

@aurorascharff @thomaslarsson Har dere noen tanker eller ev. andre forslag som unngår breaking changes?

Vi kan lage en ny mappe kalt unstable/formElements/MdInput, så bruker man denne nye og vi deprecater den gamle. Da vil kun importen endres. Vi burde gjøre det samme for MdTextArea i unstable/formElements/MdTextArea som også burde extende TextArea props. Kan du sjekke om det er flere elementer det gjelder? Så flytter vi tilbake og erstatter gamle versjon og fjerner unstable mappen igjen etterhvert. Virker som best practise er depracted -> obsolete neste versjon -> så fjerne den helt i en major. Eventuelt så kan vi droppe dette da men da må vi si ifra til alle selvsagt. Men tror et fremdeles burde være en major men da også gjøre det på flere komponenter som har samme problem (textarea).

Ja, jeg er helt enig i at det må være en major! Men jeg har bittelitt lyst på å slippe hele den runddansen med ny kopi først og deprecate + obsolete den gamle før vi er ferdig merker jeg 😓 Skjønner det er en must med f.eks. public pakker med mange bruk i forskjellige situasjoner - men er vi virkelig der med bruk av dette biblioteket? Så lenge det er en major release så tenker jeg ingen oppgraderer "ved et uhell", og fiksen - bytte size prop, er jo rimelig straight forward. Jeg kan legge til en beskrivelse ved den nye size-propen som skal brukes isteden som hjelp. Kan også ta en runde og se om det er flere komponenter som åpenbart hadde hatt nytte av den samme endringen 👍

[aurorascharff]

@aurorascharff @thomaslarsson Har dere noen tanker eller ev. andre forslag som unngår breaking changes?

Hva hvis

@aurorascharff @thomaslarsson Har dere noen tanker eller ev. andre forslag som unngår breaking changes?

Vi kan lage en ny mappe kalt unstable/formElements/MdInput, så bruker man denne nye og vi deprecater den gamle. Da vil kun importen endres. Vi burde gjøre det samme for MdTextArea i unstable/formElements/MdTextArea som også burde extende TextArea props. Kan du sjekke om det er flere elementer det gjelder? Så flytter vi tilbake og erstatter gamle versjon og fjerner unstable mappen igjen etterhvert. Virker som best practise er depracted -> obsolete neste versjon -> så fjerne den helt i en major. Eventuelt så kan vi droppe dette da men da må vi si ifra til alle selvsagt. Men tror et fremdeles burde være en major men da også gjøre det på flere komponenter som har samme problem (textarea).

Ja, jeg er helt enig i at det må være en major! Men jeg har bittelitt lyst på å slippe hele den runddansen med ny kopi først og deprecate + obsolete den gamle før vi er ferdig merker jeg 😓 Skjønner det er en must med f.eks. public pakker med mange bruk i forskjellige situasjoner - men er vi virkelig der med bruk av dette biblioteket? Så lenge det er en major release så tenker jeg ingen oppgraderer "ved et uhell", og fiksen - bytte size prop, er jo rimelig straight forward. Jeg kan legge til en beskrivelse ved den nye size-propen som skal brukes isteden som hjelp. Kan også ta en runde og se om det er flere komponenter som åpenbart hadde hatt nytte av den samme endringen 👍

Ok, enig med deg her! Så lenge vi leter etter flere komponenter det gjelder og fikser det så vi slipper flere majors 😆

[kajsaeggum]

@aurorascharff @thomaslarsson Har dere noen tanker eller ev. andre forslag som unngår breaking changes?

Hva hvis

@aurorascharff @thomaslarsson Har dere noen tanker eller ev. andre forslag som unngår breaking changes?

Vi kan lage en ny mappe kalt unstable/formElements/MdInput, så bruker man denne nye og vi deprecater den gamle. Da vil kun importen endres. Vi burde gjøre det samme for MdTextArea i unstable/formElements/MdTextArea som også burde extende TextArea props. Kan du sjekke om det er flere elementer det gjelder? Så flytter vi tilbake og erstatter gamle versjon og fjerner unstable mappen igjen etterhvert. Virker som best practise er depracted -> obsolete neste versjon -> så fjerne den helt i en major. Eventuelt så kan vi droppe dette da men da må vi si ifra til alle selvsagt. Men tror et fremdeles burde være en major men da også gjøre det på flere komponenter som har samme problem (textarea).

Ja, jeg er helt enig i at det må være en major! Men jeg har bittelitt lyst på å slippe hele den runddansen med ny kopi først og deprecate + obsolete den gamle før vi er ferdig merker jeg 😓 Skjønner det er en must med f.eks. public pakker med mange bruk i forskjellige situasjoner - men er vi virkelig der med bruk av dette biblioteket? Så lenge det er en major release så tenker jeg ingen oppgraderer "ved et uhell", og fiksen - bytte size prop, er jo rimelig straight forward. Jeg kan legge til en beskrivelse ved den nye size-propen som skal brukes isteden som hjelp. Kan også ta en runde og se om det er flere komponenter som åpenbart hadde hatt nytte av den samme endringen 👍

Ok, enig med deg her! Så lenge vi leter etter flere komponenter det gjelder og fikser det så vi slipper flere majors 😆

Har jobbet en del timer i dag med å innføre dette for alle komponenter hvor det er en tydelig og gitt "indre" html-komponent som burde få tildelt alle standard attributt. Tror det er en veldig fornuftig omskriving som gir mye mer fleksibilitet og konfigurasjonsmuligheter fra utsiden ved bruk av de. Men det blir breaking changes for flere komponenter, fremst på dette med size-prop men også id-prop som tidligere var definert med number som lovlig type.

Når jeg kom meg et stykke ned i listen så ser jeg at det noen steder er spesifisert otherProps på denne måten her, det gjør jo at man kan sende inn en hvilken som helst prop og at den ihvertfall blir forwarded til html-komponenten.

En slik liten justering på MdInput ville løst mitt opprinnelige behov med å kunne sette en haug med aria-props, uten å måtte lage noen major utrulling her og nå. Det er ikke en like komplett løsning som rydder opp i alle mulige behov for å kunne kontrollere html-elementene fra utsiden, men jeg ble plutselig litt usikker på om jeg skulle ferdigstille jobben med den store omskrivingen eller bare si seg fornøyd med en easy fix for denne gang 🙈

[aurorascharff]

@aurorascharff @thomaslarsson Har dere noen tanker eller ev. andre forslag som unngår breaking changes?

Hva hvis

@aurorascharff @thomaslarsson Har dere noen tanker eller ev. andre forslag som unngår breaking changes?

Vi kan lage en ny mappe kalt unstable/formElements/MdInput, så bruker man denne nye og vi deprecater den gamle. Da vil kun importen endres. Vi burde gjøre det samme for MdTextArea i unstable/formElements/MdTextArea som også burde extende TextArea props. Kan du sjekke om det er flere elementer det gjelder? Så flytter vi tilbake og erstatter gamle versjon og fjerner unstable mappen igjen etterhvert. Virker som best practise er depracted -> obsolete neste versjon -> så fjerne den helt i en major. Eventuelt så kan vi droppe dette da men da må vi si ifra til alle selvsagt. Men tror et fremdeles burde være en major men da også gjøre det på flere komponenter som har samme problem (textarea).

Ja, jeg er helt enig i at det må være en major! Men jeg har bittelitt lyst på å slippe hele den runddansen med ny kopi først og deprecate + obsolete den gamle før vi er ferdig merker jeg 😓 Skjønner det er en must med f.eks. public pakker med mange bruk i forskjellige situasjoner - men er vi virkelig der med bruk av dette biblioteket? Så lenge det er en major release så tenker jeg ingen oppgraderer "ved et uhell", og fiksen - bytte size prop, er jo rimelig straight forward. Jeg kan legge til en beskrivelse ved den nye size-propen som skal brukes isteden som hjelp. Kan også ta en runde og se om det er flere komponenter som åpenbart hadde hatt nytte av den samme endringen 👍

Ok, enig med deg her! Så lenge vi leter etter flere komponenter det gjelder og fikser det så vi slipper flere majors 😆

Har jobbet en del timer i dag med å innføre dette for alle komponenter hvor det er en tydelig og gitt "indre" html-komponent som burde få tildelt alle standard attributt. Tror det er en veldig fornuftig omskriving som gir mye mer fleksibilitet og konfigurasjonsmuligheter fra utsiden ved bruk av de. Men det blir breaking changes for flere komponenter, fremst på dette med size-prop men også id-prop som tidligere var definert med number som lovlig type.

Når jeg kom meg et stykke ned i listen så ser jeg at det noen steder er spesifisert otherProps på denne måten her, det gjør jo at man kan sende inn en hvilken som helst prop og at den ihvertfall blir forwarded til html-komponenten. En slik liten justering på MdInput ville løst mitt opprinnelige behov med å kunne sette en haug med aria-props, uten å måtte lage noen major utrulling her og nå. Det er ikke en like komplett løsning som rydder opp i alle mulige behov for å kunne kontrollere html-elementene fra utsiden, men jeg ble plutselig litt usikker på om jeg skulle ferdigstille jobben med den store omskrivingen eller bare si seg fornøyd med en easy fix for denne gang 🙈

Hei Kajsa! Bra du tenker på dette. Denne otherprops unknown tror jeg vi burde holde oss unna egentlig - den brukes blandt annet i ikonts for å la oss sende size ned til SVG-en fordi den feiler uten (merket det nylig). Men det vil jo si at du mister typechecking på ulovlige props helt! Så jeg skjønner hvorfor du tenkte det men jeg tror nok ikke det er lurt å innføre i input etc.

[aurorascharff]

@aurorascharff @thomaslarsson Har dere noen tanker eller ev. andre forslag som unngår breaking changes?

Hva hvis

@aurorascharff @thomaslarsson Har dere noen tanker eller ev. andre forslag som unngår breaking changes?

Vi kan lage en ny mappe kalt unstable/formElements/MdInput, så bruker man denne nye og vi deprecater den gamle. Da vil kun importen endres. Vi burde gjøre det samme for MdTextArea i unstable/formElements/MdTextArea som også burde extende TextArea props. Kan du sjekke om det er flere elementer det gjelder? Så flytter vi tilbake og erstatter gamle versjon og fjerner unstable mappen igjen etterhvert. Virker som best practise er depracted -> obsolete neste versjon -> så fjerne den helt i en major. Eventuelt så kan vi droppe dette da men da må vi si ifra til alle selvsagt. Men tror et fremdeles burde være en major men da også gjøre det på flere komponenter som har samme problem (textarea).

Ja, jeg er helt enig i at det må være en major! Men jeg har bittelitt lyst på å slippe hele den runddansen med ny kopi først og deprecate + obsolete den gamle før vi er ferdig merker jeg 😓 Skjønner det er en must med f.eks. public pakker med mange bruk i forskjellige situasjoner - men er vi virkelig der med bruk av dette biblioteket? Så lenge det er en major release så tenker jeg ingen oppgraderer "ved et uhell", og fiksen - bytte size prop, er jo rimelig straight forward. Jeg kan legge til en beskrivelse ved den nye size-propen som skal brukes isteden som hjelp. Kan også ta en runde og se om det er flere komponenter som åpenbart hadde hatt nytte av den samme endringen 👍

Ok, enig med deg her! Så lenge vi leter etter flere komponenter det gjelder og fikser det så vi slipper flere majors 😆

Har jobbet en del timer i dag med å innføre dette for alle komponenter hvor det er en tydelig og gitt "indre" html-komponent som burde få tildelt alle standard attributt. Tror det er en veldig fornuftig omskriving som gir mye mer fleksibilitet og konfigurasjonsmuligheter fra utsiden ved bruk av de. Men det blir breaking changes for flere komponenter, fremst på dette med size-prop men også id-prop som tidligere var definert med number som lovlig type.
Når jeg kom meg et stykke ned i listen så ser jeg at det noen steder er spesifisert otherProps på denne måten her, det gjør jo at man kan sende inn en hvilken som helst prop og at den ihvertfall blir forwarded til html-komponenten. En slik liten justering på MdInput ville løst mitt opprinnelige behov med å kunne sette en haug med aria-props, uten å måtte lage noen major utrulling her og nå. Det er ikke en like komplett løsning som rydder opp i alle mulige behov for å kunne kontrollere html-elementene fra utsiden, men jeg ble plutselig litt usikker på om jeg skulle ferdigstille jobben med den store omskrivingen eller bare si seg fornøyd med en easy fix for denne gang 🙈

Hei Kajsa! Bra du tenker på dette. Denne otherprops unknown tror jeg vi burde holde oss unna egentlig - den brukes blandt annet i ikonts for å la oss sende size ned til SVG-en fordi den feiler uten (merket det nylig). Men det vil jo si at du mister typechecking på ulovlige props helt! Så jeg skjønner hvorfor du tenkte det men jeg tror nok ikke det er lurt å innføre i input etc.

Vi burde nok fjerne [otherprops: string]: unknown andre steder enn icon egentlig... Men beholde ...otherprops speaden men bare ikke den unknown delen.

[kajsaeggum]

@aurorascharff @thomaslarsson Har dere noen tanker eller ev. andre forslag som unngår breaking changes?

Hva hvis

@aurorascharff @thomaslarsson Har dere noen tanker eller ev. andre forslag som unngår breaking changes?

Vi kan lage en ny mappe kalt unstable/formElements/MdInput, så bruker man denne nye og vi deprecater den gamle. Da vil kun importen endres. Vi burde gjøre det samme for MdTextArea i unstable/formElements/MdTextArea som også burde extende TextArea props. Kan du sjekke om det er flere elementer det gjelder? Så flytter vi tilbake og erstatter gamle versjon og fjerner unstable mappen igjen etterhvert. Virker som best practise er depracted -> obsolete neste versjon -> så fjerne den helt i en major. Eventuelt så kan vi droppe dette da men da må vi si ifra til alle selvsagt. Men tror et fremdeles burde være en major men da også gjøre det på flere komponenter som har samme problem (textarea).

Ja, jeg er helt enig i at det må være en major! Men jeg har bittelitt lyst på å slippe hele den runddansen med ny kopi først og deprecate + obsolete den gamle før vi er ferdig merker jeg 😓 Skjønner det er en must med f.eks. public pakker med mange bruk i forskjellige situasjoner - men er vi virkelig der med bruk av dette biblioteket? Så lenge det er en major release så tenker jeg ingen oppgraderer "ved et uhell", og fiksen - bytte size prop, er jo rimelig straight forward. Jeg kan legge til en beskrivelse ved den nye size-propen som skal brukes isteden som hjelp. Kan også ta en runde og se om det er flere komponenter som åpenbart hadde hatt nytte av den samme endringen 👍

Ok, enig med deg her! Så lenge vi leter etter flere komponenter det gjelder og fikser det så vi slipper flere majors 😆

Har jobbet en del timer i dag med å innføre dette for alle komponenter hvor det er en tydelig og gitt "indre" html-komponent som burde få tildelt alle standard attributt. Tror det er en veldig fornuftig omskriving som gir mye mer fleksibilitet og konfigurasjonsmuligheter fra utsiden ved bruk av de. Men det blir breaking changes for flere komponenter, fremst på dette med size-prop men også id-prop som tidligere var definert med number som lovlig type.
Når jeg kom meg et stykke ned i listen så ser jeg at det noen steder er spesifisert otherProps på denne måten her, det gjør jo at man kan sende inn en hvilken som helst prop og at den ihvertfall blir forwarded til html-komponenten. En slik liten justering på MdInput ville løst mitt opprinnelige behov med å kunne sette en haug med aria-props, uten å måtte lage noen major utrulling her og nå. Det er ikke en like komplett løsning som rydder opp i alle mulige behov for å kunne kontrollere html-elementene fra utsiden, men jeg ble plutselig litt usikker på om jeg skulle ferdigstille jobben med den store omskrivingen eller bare si seg fornøyd med en easy fix for denne gang 🙈

Hei Kajsa! Bra du tenker på dette. Denne otherprops unknown tror jeg vi burde holde oss unna egentlig - den brukes blandt annet i ikonts for å la oss sende size ned til SVG-en fordi den feiler uten (merket det nylig). Men det vil jo si at du mister typechecking på ulovlige props helt! Så jeg skjønner hvorfor du tenkte det men jeg tror nok ikke det er lurt å innføre i input etc.

Godt poeng! Da gjør jeg meg ferdig med hele refaktoreringen, godt med en liten bekreftelse på at det ikke gjøres unødvendig ihvertfall 😅

[aurorascharff]

Utrolig godt jobbet med denne refaktoreringen!!! Kanskje vi burde se etter flere breaking changes? Feks alle id?: string | number? Eller id?: string | number | undefined

[kajsaeggum]

Utrolig godt jobbet med denne refaktoreringen!!! Kanskje vi burde se etter flere breaking changes? Feks alle id: string | number?

Hehe, det ble litt 😅
Kan godt fjerne number-id'er også hvis det er greit at pr'en vokser!
En annen ting som jeg egentlig ville rydda unna, men holdt meg fra for å minimere changes opprinnerlig, var å fjerne ariaLabel-propen der den var eksplisitt definert. Nå vil jo heller default aria-label være mulig å bruke isteden. F.eks. i MdHelpButton og MdTooltip

[aurorascharff]

Utrolig godt jobbet med denne refaktoreringen!!! Kanskje vi burde se etter flere breaking changes? Feks alle id: string | number?

Hehe, det ble litt 😅 Kan godt fjerne number-id'er også hvis det er greit at pr'en vokser! En annen ting som jeg egentlig ville rydda unna, men holdt meg fra for å minimere changes opprinnerlig, var å fjerne ariaLabel-propen der den var eksplisitt definert. Nå vil jo heller default aria-label være mulig å bruke isteden. F.eks. i MdHelpButton og MdTooltip

Tanken med denne propen var egentlig at den skal være required (i disse tilfellene siden vi vet at det er feil UU uten), men det er kanskje ikke best practise?

[kajsaeggum]

Utrolig godt jobbet med denne refaktoreringen!!! Kanskje vi burde se etter flere breaking changes? Feks alle id: string | number?

Hehe, det ble litt 😅 Kan godt fjerne number-id'er også hvis det er greit at pr'en vokser! En annen ting som jeg egentlig ville rydda unna, men holdt meg fra for å minimere changes opprinnerlig, var å fjerne ariaLabel-propen der den var eksplisitt definert. Nå vil jo heller default aria-label være mulig å bruke isteden. F.eks. i MdHelpButton og MdTooltip

Tanken med denne propen var egentlig at den skal være required (i disse tilfellene siden vi vet at det er feil UU uten), men det er kanskje ikke best practise?

Aha, skjønner. Men det gjelder bare MdIconButton da, det var den eneste som hadden ariaLabel som ikke var optional. Kan legge tilbake den?

[aurorascharff]

Utrolig godt jobbet med denne refaktoreringen!!! Kanskje vi burde se etter flere breaking changes? Feks alle id: string | number?

Hehe, det ble litt 😅 Kan godt fjerne number-id'er også hvis det er greit at pr'en vokser! En annen ting som jeg egentlig ville rydda unna, men holdt meg fra for å minimere changes opprinnerlig, var å fjerne ariaLabel-propen der den var eksplisitt definert. Nå vil jo heller default aria-label være mulig å bruke isteden. F.eks. i MdHelpButton og MdTooltip

Tanken med denne propen var egentlig at den skal være required (i disse tilfellene siden vi vet at det er feil UU uten), men det er kanskje ikke best practise?

Aha, skjønner. Men det gjelder bare MdIconButton da, det var den eneste som hadden ariaLabel som ikke var optional. Kan legge tilbake den?

Ah, daså, ja det kan du! Har gjort det samme i helpButton forståvidt. Men der er den optional fordi vi har en default.
Ser at du har gjort den optional i MdTooltip men her burde den være required!

[crolsson]

Dere jobber bra sammen her @kajsaeggum og @aurorascharff ! Er det fortsatt usikkerhet rundt dette kan vi kanskje ta en diskusjon med flere, men ellers ser det ut for meg som dette blir bra håndtert :)

[aurorascharff]

Testet oppgradering i eget prosjekt og ting ser fint ut!