Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Heading #234

Closed
10 of 11 tasks
MrSkippy opened this issue Sep 23, 2024 · 4 comments · Fixed by #278
Closed
10 of 11 tasks

Heading #234

MrSkippy opened this issue Sep 23, 2024 · 4 comments · Fixed by #278
Assignees
Labels
new component Adding a new component to the library react Issue involving a ReactJS component

Comments

@MrSkippy
Copy link
Contributor

MrSkippy commented Sep 23, 2024

Omschrijving

Utrecht Heading naar LUX Heading
Koptekst die in de koppenstructuur ingesteld kan worden op het juiste niveau.

Features die overgenomen worden:

  • level nummers 1 tm 6; renders <h1> t/m <h6>
  • appearance nummers 1 tm 6; zet de classname utrecht-heading-1 t/m utrecht-heading-6
  • Geen of verkeerd level/appearance wordt H6

Note:
De Heading1 t/m Heading6 zijn componenten die hieruit kunnen worden ontwikkeld.
Bijvoorbeeld

export const Heading3 = <Heading level="3" appearance="3" />

Definition of done

Component

  • Component is beschikbaar in de index
  • Design tokens worden gebruikt

Storybook

  • Playground story
  • States en properties stories
  • Korte beschrijving van het component
    • Wat doet het
    • Basis van het component (bijv Utrecht)
    • Aanpassingen en/of uitbreidingen
    • Relatie met andere componenten (bijv. Form field label en Form field)

Tests

  • Het component rendert
  • Aanpassingen en/of uitbreidingen werken (zie ook Testing components)
  • Visual regression test (door LUX)

Links

Component: https://www.nldesignsystem.nl/heading (Note: ook beschikbaar bij Utrecht)
Figma NLDS met lux tokens: https://www.figma.com/design/jzxqJv4PtgHJdmOJwY6lYg/NLDS---Bibliotheek---LUX-tokens?node-id=153-1039
Utrecht: https://nl-design-system.github.io/utrecht/storybook/?path=/docs/react_react-heading--docs

@MrSkippy MrSkippy converted this from a draft issue Sep 23, 2024
@rwittenberns
Copy link
Contributor

rwittenberns commented Sep 24, 2024

We willen 2 features op een heading component: level en appearance zodat we zowel html element en stijling apart kunnen instellen. Nagaan of dit bij Utrecht Heading component mogelijk is. Aangeven bij Utrecht dat Heading component mogelijk herbruikbaarder kunnen opzetten.

@MrSkippy
Copy link
Contributor Author

Vragen voor de planning?

  • willen we voor appearance-waarden "utrecht-heading-1" etc. of "1" t/m "6". Imho de laatste is beter ivm dat het later de class name nl-heading-1 wordt.
  • hoe gaan we NLdoc's oneindige headings oplossen? Utrecht maakt er altijd een H1 van, NLdoc wil dat een H8 (iets wat in Word kan...) een H6 wordt.

@rwittenberns
Copy link
Contributor

Vragen voor de planning?

  • willen we voor appearance-waarden "utrecht-heading-1" etc. of "1" t/m "6". Imho de laatste is beter ivm dat het later de class name nl-heading-1 wordt.
    LUX: voorstel is vendor agnostisch ("heading-1" of "1"). Om breaking change te voorkomen beide ondersteunen. Voor LUX oplossen maar ook bij Utrecht discussie starten over gewenste wijziging.
  • hoe gaan we NLdoc's oneindige headings oplossen? Utrecht maakt er altijd een H1 van, NLdoc wil dat een H8 (iets wat in Word kan...) een H6 wordt.
    Max level is altijd 6. Appearance bepaalt dan het zichtbare verschil.

@MrSkippy MrSkippy added new component Adding a new component to the library react Issue involving a ReactJS component labels Oct 1, 2024
@MMeijerink MMeijerink self-assigned this Oct 3, 2024
@remypar5
Copy link
Contributor

remypar5 commented Oct 3, 2024

We willen 2 features op een heading component: level en appearance zodat we zowel html element en stijling apart kunnen instellen. Nagaan of dit bij Utrecht Heading component mogelijk is. Aangeven bij Utrecht dat Heading component mogelijk herbruikbaarder kunnen opzetten.

Ik ben voor om de appearance optioneel te maken. Als standaard moet die de zelfde waarde hebben als level. Die is dus alléén nodig als level en appearance van elkaar afwijken.

@MMeijerink MMeijerink linked a pull request Oct 7, 2024 that will close this issue
3 tasks
@github-project-automation github-project-automation bot moved this from In code review to Done in Lux React Components Oct 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new component Adding a new component to the library react Issue involving a ReactJS component
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants