-
Notifications
You must be signed in to change notification settings - Fork 1
feat(button): HTML element implementatie (#784) #831
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
base: main
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #831 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 28 28
Lines 310 312 +2
Branches 52 52
=========================================
+ Hits 310 312 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This way you can still follow BEM best practices wihout unwanted side effects. `<div class="nl-button nl-button--html"><button>...` would not work. `<div class="nl-html nl-html--button"><button>...` would work.
|



Naar de instructies in de issue, aan de hand van de CSS die wij hebben, en vervolgens vergeleken met Utrecht.
Stories in Storybook:
Stories in Storybook Test:
Daarnaast adhv de issue ook nl-button--submit toegevoegd (in CSS, Test, React, Stories). Deze moet nog even besproken worden met Robbert of het zo de bedoeling is. Dit stond in de issue, maar niet in onze acceptatiecriteria, en dan zou er ook nog een token missen / moeten we beslissen of het hardcoded CSS is of niet.
✅ Die willen we toch niet! Verwijderd.
Voor de mixin nl-button--html heb ik uitgebreider gewerkt dan Utrecht: Utrecht styled maar 1x op inputs zowel als button, en verder enkel op de button. Ik heb op zowel de inputs als de button gestyled.
Daarnaast gebruik ik
:wherezoals we ook doen in de rest van onze Button CSS. Utrecht doet dit niet.De stories die hergebruikt zijn, weergeven hun naam in de Storybook Sidebar als de const name in plaats van de name property. Lijkt altijd zo te gaan, ondanks wat het internet zegt. Uitgeprobeerd:
Beiden geven "Button".
Maar in de Storybook Sidebar krijg ik altijd "Default":
Soms lijkt dit wel goed te gaan in de normale Storybook, bijvoorbeeld voor de "Volle Breedte" story.
Daarnaast heb ik een button-html.tsx in de storybook-test gezet, om een component te maken welke button rendered met alle gewenste attributen, zonder alle extra functionaliteiten en classes. Is dat gewenst zo, of liever de Button component wrappen en de classes eraf strippen?
In button.stories.tsx heb ik de stories met custom render uitgewerkt zodat het werkt met de ingestelde button in de meta ipv altijd de Button zoals geimporteerd in button.stories.tsx. Dit helpt me met Stories die de React Button gebruiken versus de HTML Button gebruiken. Dit is afgekeken van hoe we het ook voor Link en Mark hebben gedaan.
In de templates heb ik alvast standaard de --html suffix toegevoegd.