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

chore(storybook): 🪧 Add fieldset preview #2047

Merged
merged 2 commits into from
May 23, 2024

Conversation

poi33
Copy link
Collaborator

@poi33 poi33 commented May 21, 2024

Was working on the fieldset and saw there was no preview in storybook.

@poi33 poi33 self-assigned this May 21, 2024
@poi33 poi33 requested review from mimarz and Barsnes as code owners May 21, 2024 11:11
Copy link
Collaborator

@mimarz mimarz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! We could probably use some Textfield and Textarea inside it for the preview as its what most intended to use purely with this component.

Checkbox and Radio have their own .Group components which are just extension of this Fieldset with additional context features for those specifc form elements.

export const Preview: Story = (args) => (
<form>
<Fieldset {...args}>
<RadioGroup legend=''>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This RadioGroup is redundant here as its the same Fieldset component just with some additional props for handling radios in groups, such as name.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I'm making this mistake, I would guess a lot more will use the redundant grouping too?
I noticed that the radio group got a lock icon when in read only and the fieldset did too so something was not correct.

Copy link
Collaborator

@mimarz mimarz May 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its probably due to them using the same React context, since its passed there to all child components, which includes all components under components/form

Good point on the need to clarify use of RadioGroup and Fieldset. My guess its best to document this in the ingress for the Component in Storybook and jsdoc.

@poi33 poi33 force-pushed the chore/fieldset-preview branch from f5d8250 to f443292 Compare May 22, 2024 10:32
@poi33 poi33 force-pushed the chore/fieldset-preview branch from f443292 to d69e8c0 Compare May 22, 2024 10:46
@poi33 poi33 requested a review from mimarz May 22, 2024 10:46
Co-authored-by: Michael Marszalek <mimarz@gmail.com>
@mimarz mimarz merged commit 17f19f2 into digdir:main May 23, 2024
2 of 3 checks passed
@poi33 poi33 deleted the chore/fieldset-preview branch May 23, 2024 09:40
TobiasGunther pushed a commit to kjetil-hoel/designsystem that referenced this pull request May 28, 2024
Co-authored-by: Michael Marszalek <mimarz@gmail.com>
TobiasGunther added a commit to kjetil-hoel/designsystem that referenced this pull request May 28, 2024
* feat(ErrorMessage): add shorthand sizes (digdir#2019)

* feat(Textfield): add shorthand sizes (digdir#2018)

* feat(Switch): add shorthand sizes (digdir#2027)

* feat(Search): add shorthand sizes (digdir#2028)

* feat(Checkbox): add shorthand sizes (digdir#2030)

* feat(Textarea): add shorthand sizes (digdir#2031)

* feat(Combobox): add shorthand sizes (digdir#2029)

* feat(Ingress): add shorthand sizes (digdir#2020)

* feat(Chips): add shorthand sizes (digdir#2007)

Co-authored-by: Michael Marszalek <mimarz@gmail.com>

* feat(Tag): add shorthand sizes (digdir#2017)

* feat(Table): add shorthand sizes (digdir#2016)

* feat(HelpText): add shorthand sizes (digdir#2014)

* feat(DropdownMenu): add shorthand sizes (digdir#2021)

* feat(ToggleGroup): add shorthand sizes (digdir#2011)

Co-authored-by: Michael Marszalek <mimarz@gmail.com>

* feat(Tabs): add shorthand sizes (digdir#2012)

Co-authored-by: Michael Marszalek <mimarz@gmail.com>

* feat(List): add shorthand sizes (digdir#2009)

Co-authored-by: Michael Marszalek <mimarz@gmail.com>

* feat(Alert): add shorthand sizes (digdir#1995)

Co-authored-by: Michael Marszalek <mimarz@gmail.com>

* feat(NativeSelect): add shorthand sizes (digdir#2033)

* feat(Radio): add shorthand sizes (digdir#2036)

* feat(Pagination): add shorthand sizes (digdir#2034)

* chore: sort `fds.box` layer (digdir#2051)

* chore: update clsx import (digdir#2049)

* chore: move storybook to `apps` (digdir#2052)

* feat(Box): add shorthand sizes (digdir#2048)

* chore: cleanup after size changes (digdir#2043)

* chore: storybook introduction (digdir#2058)

Co-authored-by: Michael Marszalek <mimarz@gmail.com>

* fix(Tabs): `className` being overridden (digdir#2064)

* fix(Tabs): active tab has wrong hover style (digdir#2061)

* fix(Combobox): remove wrong padding (digdir#2067)

* chore(storybook): 🪧 Add fieldset preview (digdir#2047)

Co-authored-by: Michael Marszalek <mimarz@gmail.com>

* docs: sizing guidelines for Button (digdir#2013)

Co-authored-by: Tobias Barsnes <tobias.barsnes@digdir.no>
Co-authored-by: Michael Marszalek <mimarz@gmail.com>

* fix(RadioGroup): bigger horizontal gap between inputs (digdir#2068)

* chore: add all tsconfigs to eslint and fix alias (digdir#2069)

* fix(Tooltip): rendered in a portal by default (digdir#2060)

Co-authored-by: Michael Marszalek <mimarz@gmail.com>

* fix(Textfield): character counter inital value (digdir#2056)

* Publish

 - @digdir/designsystemet-css@0.9.0
 - @digdir/designsystemet-react@0.62.0
 - @digdir/design-system-react@0.53.12

* build: ⬆️ Update Storybook to 8.1.3 (digdir#2072)

* build: ⬆️ Update Yarn to 4.2.2 (digdir#2071)

* fix(Heading): ⚰️ Remove non-working `3xlarge`/`3xl` size (digdir#2074)

---------

Co-authored-by: Tobias Barsnes <tobias.barsnes@digdir.no>
Co-authored-by: Michael Marszalek <mimarz@gmail.com>
Co-authored-by: Persijn kwekkeboom <3340349+poi33@users.noreply.github.com>
Co-authored-by: Marianne Røsvik <marianne.rosvik@digdir.no>
Co-authored-by: paal2707 <68380161+paal2707@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants