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.

Sign up for GitHub

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

Jump to bottom

feat(Form): use nuxt useId to bind input labels #1211

Merged
merged 18 commits into from
Jan 31, 2024
Merged

feat(Form): use nuxt useId to bind input labels #1211

merged 18 commits into from
Jan 31, 2024

Conversation

romhml
Copy link
Collaborator

@romhml romhml commented Jan 7, 2024 •
edited
Loading

🔗 Linked issue

#1171

❓ Type of change

  • 📖 Documentation (updates to the documentation or readme)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • 👌 Enhancement (improving an existing functionality)
  • ✨ New feature (a non-breaking change that adds functionality)
  • 🧹 Chore (updates to the build process or auxiliary tools and libraries)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

📚 Description

Replaces our custom unique id generation by nuxt's useId to bind accessibility attribute in a SSR friendly way.

📝 Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

@vercel Vercel
Copy link

vercel bot commented Jan 7, 2024 •
edited
Loading

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

Name Status Preview Updated (UTC)
ui ✅ Ready (Inspect) Visit Preview Jan 31, 2024 2:57pm

@romhml romhml changed the title feat(Form): use useId to bind input labels feat(Form): use nuxt's useId to bind input labels Jan 8, 2024
@benjamincanac
Copy link
Member

@romhml https://nuxt.com/blog/v3-10#ssr-safe-accessible-unique-id-creation 🔥 I've updated nuxt to 3.10.0 on dev branch!

@romhml
Copy link
Collaborator Author

romhml commented Jan 30, 2024

Great 🚀! I'll wrap this up!

@benjamincanac Volta.net
Copy link
Member

Let me know how it goes! On my side, I'll try to use this composable to fix all the Headless UI hydration issues since Vue 3.4.

@romhml
fix: useId key must be a string
ca2c3c3 
- Removed nuxt dependency overrides to fix useId errors
- Updated useId imports to "#app"
@romhml
chore: cleanup playground
9abd132
@romhml
Copy link
Collaborator Author

romhml commented Jan 31, 2024 •
edited
Loading

@benjamincanac I had to remove the dependencies overrides for nuxt/kit and nuxt/schema for useId to fix this error. Also unpinned vue since we should not have any more hydration issues, which caused type issues. Do you have any insights on this? Especially this one: https://github.com/nuxt/ui/actions/runs/7719033426/job/21041516307?pr=1211#step:11:16

@ybw0014 ybw0014 mentioned this pull request Jan 31, 2024
Closed
@benjamincanac Volta.net
Copy link
Member

I've updated the resolutions, sorry I missed that on the upgrade! The typecheck errors should be gone too if you update your branch from dev!

The hydration issues won't be fixed until we take advantage of useId and pass it to all Headless UI components.. I'll try to do this asap!

@romhml romhml marked this pull request as ready for review January 31, 2024 14:32
benjamincanac
benjamincanac reviewed Jan 31, 2024
View reviewed changes
src/runtime/components/forms/Form.vue Outdated
@@ -1,18 +1,18 @@
<template>
<form @submit.prevent="onSubmit">
<form :id="formId" @submit.prevent="onSubmit">
Copy link
Member

Choose a reason for hiding this comment

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

Why add an id to the form here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I removed it, it was a leftover from a test to check for hydration issues.

benjamincanac
benjamincanac reviewed Jan 31, 2024
View reviewed changes
Copy link
Member

@benjamincanac benjamincanac left a comment

Choose a reason for hiding this comment

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

The ids look like this in the HTML, should we define our own key like form or radio? We can also leave it as it, not really sure.
CleanShot 2024-01-31 at 16 28 23@2x

@benjamincanac benjamincanac changed the title feat(Form): use nuxt's useId to bind input labels chore(Form): use nuxt useId to bind input labels Jan 31, 2024
@benjamincanac benjamincanac changed the title chore(Form): use nuxt useId to bind input labels feat(Form): use nuxt useId to bind input labels Jan 31, 2024
@benjamincanac benjamincanac merged commit 27c71fa into dev Jan 31, 2024
2 checks passed
@benjamincanac benjamincanac deleted the feat-form-useid branch January 31, 2024 17:22
@benjamincanac Volta.net
Copy link
Member

Thanks @romhml! 😊

@Youhan
Copy link
Contributor

Youhan commented Feb 21, 2024

Sorry for spamming. Just wondering if there is a way to see when this will be shipped. Is there a way to check the version this will be shipped at?

@benjamincanac Volta.net
Copy link
Member

benjamincanac commented Feb 21, 2024 •
edited
Loading

This will be released tomorrow in the v2.14.0.

@benjamincanac benjamincanac mentioned this pull request Feb 26, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@benjamincanac benjamincanac benjamincanac approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@romhml @benjamincanac @Youhan