Content and form page style refactor with utrecht components #268

dutchcelt · 2022-09-21T13:16:48Z

Covered items in two pages: Content and Formulieren.
The styles have been split out into mixins.

See issue: #265

dutchcelt · 2022-09-26T09:19:01Z

Added breadcrumb component to this PR. Probably should have been separate PR.

sushitommy

Hey Egor, nice work! Ik zie dat je een hoop dingen gefixt hebt. En mooi te zien dat het goed werkt met de React components van Utrecht. Ik heb nog wat vragen/puntjes, zie de review. En wellicht ook goed om nog even kort wat dingen door te spreken (later deze week)?

sushitommy · 2022-10-04T13:43:26Z

components/button/css/template.tsx

+          ? 'subtle-button'
+          : null
+      }
+      hint={kind === 'warning' || kind === 'warning-subtle' ? 'warning' : null}
    >
      {showIcon === 'before' && iconMarkup}


Ik krijg een lint error: This JSX tag's 'children' prop expects a single child of type 'ReactNode', but multiple children were provided. Misschien Fragment gebruiken?

We zouden idd een fragment kunnen toevoegen. Ik test het wel eerst.

I think it might have to do with children?: React.ReactNode;, allowing one instead of many children. I have updated all the property definitions to use PropsWithChildren from React to fix this.

sushitommy · 2022-10-04T13:53:57Z

components/form-field/css/template.tsx

-        <span className="rvo-form-field__label-text">{labelText}</span>
+    <FormField className="rvo-form-field rvo-layout-column rvo-layout-gap--sm">
+      <div className="rvo-form-field__label rvo-layout-column rvo-layout-gap--2xs">
+        <FormLabel className="rvo-form-field__label-text" htmlFor={fieldId}>


Ik krijg hier een lint error 'Property 'className' does not exist on type 'FormLabelProps & { children?: ReactNode; } & RefAttributes'

Wellicht zie ik iets over het hoofd, ik kan deze niet reproduceren. Is er een specifiek lint script die ik kan draaien?

@tomjonkman Ik heb tsc --noEmit toegevoegd als extra check om voor dit soort fouten de required PR checks te laten falen, maar ook daar komt deze warning niet naar boven... (en className zou gewoon in de props horen te zitten, als ik zo naar de FormLabel code van Utrecht kijk)

sushitommy · 2022-10-04T14:12:56Z

components/icon/css/template.tsx

@@ -13,7 +13,7 @@ interface IIconProps {
  classNames?: string[];
 }

-export const iconColors = ['hemelblauw', 'wit', 'zwart'];
+export const iconColors = ['', 'hemelblauw', 'wit', 'zwart'];


Waarom heb je de lege optie toegevoegd?

Dit is een change van @Robbert, ik zal het even nalopen.

De default is nu dat de icon var(--utrecht-icon-color) gebruikt currentColor en de kleur van de omgeving aanneemt, daarvoor is het color argument optioneel. Op deze manier kunnen componenten via --utrecht-icon-color: currentColor de kleur van de icon beinvloeden zonder de preciese selector te weten waarvan de background-color aangepast moet worden.

We kunnen de icon color voor stories op zich standaard instellen op hemelblauw, maar ik wil niet de indruk wekken dat een kleur kiezen verplicht is.

sushitommy · 2022-10-04T14:21:13Z

components/tag/css/index.scss

-.rvo-tag--default .rvo-icon {
-  background-color: var(--rvo-tag-default-icon-color);
-}
-
 // Info
 .rvo-tag--info {


De icons in Tag krijgen nog niet de juiste kleur, klopt dat?

Ik zal deze ook nalopen.

sushitommy · 2022-10-04T14:30:46Z

documentation/demopages/nl-design-system/common/mixins/_alerts.scss

@@ -0,0 +1,70 @@
+@mixin alerts {


Algemeen puntje: Ik ben benieuwd waarom deze mixins op deze manier en plek zijn gedefinieerd. En waarom gebruiken we hier niet de design tokens zoals gedefinieerd in alert.tokens.json?

Ik heb de opzet van de demopages/common/style.scss bestand overgenomen en opgesplitst in losse bestanden om ze 1 voor 1 over te zetten of om te zetten in bestaande design system oplossingen. De mixin aanpak is zodat ik een ander base class kan gebruiken naast de bestaande opzet. Zo voorkom ik ook duplicaat code in storybook tijdens ontwikkelen. (Ik zag de originele RVO styling de NL DS styling overschrijven in Storybook terwijl de RVO code niet werd aangeroepen.)

Het lijkt er nu wel op dat ik de mixins niet meer nodig heb. Dus heb ik het behoorlijk terug kunnen brengen.

vercel · 2022-10-05T14:28:19Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated
rvo	❌ Failed (Inspect)		Oct 8, 2022 at 11:20AM (UTC)

dutchcelt force-pushed the feature/rvo-style-refactor-with-utrecht-components branch 5 times, most recently from b01e2df to 4b7ec81 Compare September 26, 2022 09:10

dutchcelt force-pushed the feature/rvo-style-refactor-with-utrecht-components branch 5 times, most recently from 8469b91 to b2ec213 Compare September 29, 2022 12:28

Robbert force-pushed the feature/rvo-style-refactor-with-utrecht-components branch from b2ec213 to d871588 Compare September 30, 2022 10:12

dutchcelt force-pushed the feature/rvo-style-refactor-with-utrecht-components branch 2 times, most recently from 3f05e3f to caef8f6 Compare October 3, 2022 12:38

Robbert mentioned this pull request Oct 3, 2022

feat(form): refactored the form with utrecht components #267

Closed

Robbert requested a review from sushitommy October 3, 2022 14:20

Robbert force-pushed the feature/rvo-style-refactor-with-utrecht-components branch 6 times, most recently from 721bf31 to 67d2502 Compare October 4, 2022 06:45

sushitommy approved these changes Oct 4, 2022

View reviewed changes

Robbert force-pushed the feature/rvo-style-refactor-with-utrecht-components branch from e3d1cc9 to 76e44a6 Compare October 5, 2022 13:59

vercel bot had a problem deploying to Preview October 5, 2022 14:31 Failure

vercel bot had a problem deploying to Preview October 6, 2022 09:02 Failure

dutchcelt force-pushed the feature/rvo-style-refactor-with-utrecht-components branch from f0dcca5 to b7d2d80 Compare October 6, 2022 10:24

vercel bot had a problem deploying to Preview October 6, 2022 10:26 Failure

Robbert force-pushed the feature/rvo-style-refactor-with-utrecht-components branch from b7d2d80 to 37ece45 Compare October 6, 2022 12:20

Robbert and others added 24 commits October 7, 2022 14:20

chore: remove unecessary import of CSS component library

e6016c8

refactor: use Icon component everywhere

d3e3dae

refactor: use utrecht-icon

f251959

fix: missing token for description

cfbcc8c

fix: list initial indent

ea70fdc

refactor: use utrecht-button mixin for tertiary/quaternary

5b6dc4d

chore: remove ::before and ::after selectors for icons

b7074f4

refactor: switch to utrecht-blockquote

3ad7fb4

style: textarea tokens

125cb7a

chore: use the Button component in more places

87d84d7

fix: bad merge

936f14b

style: split out the description

67bca3d

fix: style alignment bug for icons

65e4aad

Remove nested ternaries for readability

75d0989

Fix lint issue with key prop (quick fix for now)

1f206ee

Fix another key prop lint issue

7714d79

Add hideFields option to fielset (for UXPin)

3e39386

Remove nested ternary in Table

a6efd21

ci: add TypeScript checks as lint step

690b100

chore: fix TypeScript warnings

b774532

fix: children should allow more than one child node

cf46a5d

fix: removed mixins and only using rvo styles and component patches

34291e3

ci: build before lint for TypeScript

cba1ebf

ci: ignore dist for HTML linting to fix build

c757b77

Robbert force-pushed the feature/rvo-style-refactor-with-utrecht-components branch from 7097850 to c757b77 Compare October 7, 2022 12:20

vercel bot had a problem deploying to Preview October 7, 2022 12:20 Failure

fix: lint issues introduced by cf18d4a

b7a6125

vercel bot had a problem deploying to Preview October 8, 2022 11:20 Failure

Robbert merged commit c3e2a43 into main Oct 10, 2022

Robbert deleted the feature/rvo-style-refactor-with-utrecht-components branch October 10, 2022 08:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Content and form page style refactor with utrecht components #268

Content and form page style refactor with utrecht components #268

dutchcelt commented Sep 21, 2022 •

edited

Loading

dutchcelt commented Sep 26, 2022

sushitommy left a comment

sushitommy Oct 4, 2022

dutchcelt Oct 5, 2022

Robbert Oct 5, 2022

sushitommy Oct 4, 2022

dutchcelt Oct 5, 2022

Robbert Oct 5, 2022 •

edited

Loading

sushitommy Oct 4, 2022

dutchcelt Oct 5, 2022

Robbert Oct 5, 2022

sushitommy Oct 4, 2022

dutchcelt Oct 5, 2022

sushitommy Oct 4, 2022

dutchcelt Oct 5, 2022

dutchcelt Oct 6, 2022

vercel bot commented Oct 5, 2022 •

edited

Loading

Content and form page style refactor with utrecht components #268

Content and form page style refactor with utrecht components #268

Conversation

dutchcelt commented Sep 21, 2022 • edited Loading

dutchcelt commented Sep 26, 2022

sushitommy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Robbert Oct 5, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vercel bot commented Oct 5, 2022 • edited Loading

dutchcelt commented Sep 21, 2022 •

edited

Loading

Robbert Oct 5, 2022 •

edited

Loading

vercel bot commented Oct 5, 2022 •

edited

Loading