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

Removable chips type now defaults to button #2124

Merged
merged 6 commits into from
Aug 3, 2023

Conversation

andnorda
Copy link
Collaborator

@andnorda andnorda commented Aug 1, 2023

I HTML er buttons by default type="submit", noe som gjør at removable chips i combobox submitter skjemaet de ligger i. Vet ikke om dette er den beste løsningen, men det burde fikse problemet. :)

@andnorda andnorda requested a review from KenAJoh August 1, 2023 13:43
@changeset-bot
Copy link

changeset-bot bot commented Aug 1, 2023

🦋 Changeset detected

Latest commit: 984d4f6

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 7 packages
Name Type
@navikt/ds-react Patch
@navikt/ds-css Patch
@navikt/ds-tokens Patch
@navikt/ds-tailwind Patch
@navikt/aksel-icons Patch
@navikt/aksel Patch
@navikt/aksel-stylelint Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@andnorda andnorda requested a review from it-vegard August 1, 2023 13:44
@github-actions
Copy link
Contributor

github-actions bot commented Aug 1, 2023

Endringer til review: 7

d82bf4f9f | 46 komponenter | 289 stories

@it-vegard
Copy link
Contributor

Nok en grunn til at vi burde bygge noen eksempel-sider (inkludert skjema-maler) vi kan teste komponentene våre i, for dette er ikke første gangen vi har bommet på noe sånt. 🤦 Tror til og med vi diskuterte dette på et tidspunkt...

Har selv vurdert å foreslå type="button" som default på Button-komponenten, så er enig i å i hvert fall innføre det her.
Om vi bare ønsket å fikse problemet for Combobox, så kunne vi nok lagt på type i SelectedOptions.tsx, men jeg ser egentlig ingen grunn til å ikke legge det direkte inn i Chips.Removable.

it-vegard
it-vegard previously approved these changes Aug 1, 2023
@andnorda
Copy link
Collaborator Author

andnorda commented Aug 2, 2023

Jeg synes en helt generell Button-komponent burde holde seg til specen og defaulte til type="submit" selv om det er en gotcha. Å overstyre standard oppførsel på komponenter som er tynne wrappers rundt HTML-elementer blir en gotcha i seg selv for utviklere som er vandt til å utvikle for webben. Da har man plutselig to steder man må sjekke før man kan være trygg på en komponent: MDN og Aksel.

KenAJoh
KenAJoh previously approved these changes Aug 2, 2023
KenAJoh and others added 3 commits August 2, 2023 11:04
…ld always be a button, and it is not currently used in any other way.

The reason for making this change now, is that we put type="button" on the Chip.Removable to prevent a submit event when the chip is clicked inside a form.
@it-vegard it-vegard merged commit a82bccf into main Aug 3, 2023
2 checks passed
@it-vegard it-vegard deleted the removable-chips-type-button branch August 3, 2023 09:41
@github-actions github-actions bot mentioned this pull request Aug 3, 2023
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.

3 participants