FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix(Select/SelectMenu): prevent field group context leaking into trigger slot by luckasdc · Pull Request #6946 · nuxt/ui · GitHub

/ ui Public

fix(Select/SelectMenu): prevent field group context leaking into trigger slot - #6946

Open
luckasdc wants to merge 2 commits into
nuxt:v4from
luckasdc:fix/select-field-group-trigger-slots
ConversationCommits (2)ChecksFiles changed
Open

fix(Select/SelectMenu): prevent field group context leaking into trigger slot#6946
luckasdc wants to merge 2 commits into
nuxt:v4from
luckasdc:fix/select-field-group-trigger-slots

Conversation

luckasdc commented Sep 14, 2026

Copy link
Copy Markdown

🔗 Linked issue

Resolves #6945

❓ 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

When a USelect or USelectMenu is placed inside a UFieldGroup, components rendered in the trigger's #leading, #default and #trailing slots inherit the field group context.

For example, a UBadge in the #default slot receives the horizontal field group classes (not-only:first:rounded-e-none …). Because it is the first of several children inside the trigger, its right corners become square:

<UFieldGroup>
  <USelectMenu v-model="value" :items="items">
    <template #default="{ modelValue }">
      <UBadge label="V1" />
      <span>{{ modelValue }}</span>
    </template>
  </USelectMenu>
  <UInput />
</UFieldGroup>

#6313 added FieldGroupReset to stop this leak for portal content, but the trigger slots were not covered.

This PR wraps the trigger content of Select and SelectMenu in FieldGroupReset:
- The trigger itself still receives the field group orientation and size. useFieldGroup() runs in the component's setup, above the reset.
- Components inside the trigger slots no longer inherit the group's orientation or size, which matches how #6313 treats portal content.
- The built-in clear button already receives size explicitly, so its size is unchanged.
- FieldGroupReset renders no DOM, so the existing snapshots are unchanged.

Tests: added does not leak field group context into trigger slots to Select.spec.ts and SelectMenu.spec.ts. They fail without the fix and pass with it, in both the nuxt and vue projects.

📝 Checklist

- [x] I have linked an issue or discussion.
- [ ] I have updated the documentation accordingly.

github-actions Bot added the v4 #4488 label Sep 14, 2026

coderabbitai Bot commented Sep 14, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info ⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 3a5c859c-e259-49e6-b75c-4c3e44e1370f

📥 Commits

Reviewing files that changed from the base of the PR and between 6b4d2c7 and e48068c.

📒 Files selected for processing (4)
  • src/runtime/components/Select.vue
  • src/runtime/components/SelectMenu.vue
  • test/components/Select.spec.ts
  • test/components/SelectMenu.spec.ts

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.

📝 Walkthrough

Walkthrough

SelectTrigger and ComboboxTrigger now wrap their content in FieldGroupReset. Tests verify that field-group classes remain on the trigger and do not apply to badges rendered through trigger slots.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 1add9

The select triggers retain their field-group styling while slot content is isolated, with focused coverage for both components.

🚥 Pre-merge checks | ✅ 7 ✅ Passed checks (7 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the Select and SelectMenu bug fix and describes the prevented field group context leak.
Description check ✅ Passed The description directly explains the bug, the FieldGroupReset fix, expected behavior, and test coverage.
Linked Issues check ✅ Passed The description links the pull request to issue #6945 and states that the issue is resolved.
Out of Scope Changes check ✅ Passed The changes remain within scope. They update Select and SelectMenu trigger rendering and add focused regression tests.
Linked Issues check ✅ Passed The PR satisfies issue #6945. Select.vue and SelectMenu.vue wrap leading, default, and trailing trigger content in FieldGroupReset. The trigger remains outside the reset wrapper, so it keeps the…
Out of Scope Changes check ✅ Passed All changed source code directly fixes the field-group context leak described in issue #6945. The tests provide targeted coverage for both affected components. No unrelated source, public API, DOM, or…
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2…
✨ Finishing Touches 🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

codspeed Bot commented Sep 14, 2026 •
edited
Loading

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 31 untouched benchmarks

Comparing luckasdc:fix/select-field-group-trigger-slots (1add948) with v4 (a3e64fb)

pkg-pr-new Bot commented Sep 14, 2026 •
edited
Loading

Copy link
Copy Markdown 
npm i https://pkg.pr.new/@nuxt/ui@6946

commit: 1add948

benjamincanac requested changes Sep 21, 2026
View reviewed changes

benjamincanac left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

What do you think about resetting the context directly in useFieldGroup instead? 🤔 The leak isn't specific to Select and SelectMenu, it happens in every component that calls it like Button, Badge, Input or InputMenu. Adding this right after the inject fixes them all without touching the templates, and wrappers like UTooltip or UDropdownMenu still propagate since they don't call it:

provide(fieldGroupInjectionKey, computed(() => ({ size: undefined, orientation: undefined })))

This branch was successfully deployed

1 active deployment
Preview – ui — 1add948a Deployed Sep 15, 2026 by vercel[bot]
View deployment
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

benjamincanac benjamincanac requested changes

Assignees

No one assigned

Labels

v4 #4488

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

SelectMenu/Select: FieldGroup orientation leaks into trigger slot content

2 participants

Back | FazBrowse Home | New Git URL