Open
Conversation
|
Size Change: 0 B Total Size: 7.82 MB ℹ️ View Unchanged
|
|
Flaky tests detected in 1091be5. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/25126871746
|
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
ciampo
approved these changes
Apr 30, 2026
Contributor
ciampo
left a comment
ciampo
left a comment
There was a problem hiding this comment.
LGTM 🚀
Left a bunch of inline comments, but none of them is blocking.
Feel free to merge once all pending feedback is addressed
| </Select.Trigger> | ||
| <Select.Popup> | ||
| <SelectControlSizeContext.Provider value={ size }> | ||
| { children || |
Contributor
There was a problem hiding this comment.
Nit: currently an explicitly falsy children (false, null) would silently regenerates items from items — is that a side effect we want to keep?
Alternatively we could swap this check to be explicitly about children !== undefined
Contributor
There was a problem hiding this comment.
Some additional tests we may want to add:
- A test confirming
SelectControl.Iteminherits size fromSelectControlvia context (the main reason for the wrapper component). - A test for the auto-rendered placeholder pattern (
disableditem with value:''shouldn't be selectable). - A test for
triggerContentreceiving the current value as the render-prop argument.
| disabled?: boolean; | ||
| }; | ||
|
|
||
| export type SelectControlProps = Omit< SelectRootProps, 'items' | 'inputRef' > & |
Contributor
There was a problem hiding this comment.
Just to confirm — we're ok with SelectControl inheriting all of the advanced props from Select.Root, or is there anything we feel like excluding for this higher-level version?
| import { SelectControl as _SelectControl } from './select-control'; | ||
| import { Item } from './item'; | ||
|
|
||
| Item.displayName = 'SelectControl.Item'; |
Contributor
There was a problem hiding this comment.
Should we do the same for _SelectControl?
Suggested change
| Item.displayName = 'SelectControl.Item'; | |
| _SelectControl.displayName = 'SelectControl'; | |
| Item.displayName = 'SelectControl.Item'; |
| * triggerContent={ ( value ) => ( | ||
| * <span> | ||
| * <Icon icon={ wordpress } /> | ||
| * { value } |
Contributor
There was a problem hiding this comment.
Assuming that consumers would likely display a label more than the raw value — should we update this code snippet and the WithCustomTriggerAndItems storybook example to show that the value returned by triggerContent's callback can be used to display the label?
Comment on lines
+24
to
+28
| * When showing a "placeholder" item, prefer a concise label such as "Select" | ||
| * without a trailing ellipsis. By default, items with an empty string value | ||
| * will be rendered with a lighter color in the trigger. | ||
| * | ||
| * It is recommended to also add `disabled` to the placeholder item so it cannot be reselected. |
Contributor
There was a problem hiding this comment.
Should we add this guidance directly to the JSDoc of SelectControl ? It feels like an important tip that would be very useful to consumers
|
|
||
| - Add `Drawer` primitive ([#76690](https://github.com/WordPress/gutenberg/pull/76690)). | ||
| - Add `Autocomplete` primitive ([#77642](https://github.com/WordPress/gutenberg/pull/77642)). | ||
| - Add `SelectControl` component ([#77809](https://github.com/WordPress/gutenberg/pull/77809)). |
Contributor
There was a problem hiding this comment.
We'll likely need a rebase to move the entry to the new UNRELEASED section
Comment on lines
+7
to
+10
| /** | ||
| * A complete select field with integrated label and description. | ||
| */ | ||
| export const SelectControl = forwardRef< |
Contributor
There was a problem hiding this comment.
Should the JSDoc (for SelectControl and Item) be added in the Object.assign call in index.ts ?
|
|
||
| export const SelectControl = Object.assign( _SelectControl, { | ||
| Item, | ||
| } ); |
Contributor
There was a problem hiding this comment.
Should we align this implementation with Button especially when it comes to the type assignment?
gutenberg/packages/ui/src/button/index.ts
Lines 4 to 16 in 253bb2a
Whichever we choose (explicit type casting or implicit), we should use it consistently. Possible, we could also document the difference between "real" compound components (using barrel export) vs higher level components like SelectControl and Button (using Object.assign) in the package's Guidelines?
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What?
Adds a new
SelectControlcomponent to@wordpress/ui, composed from the existing primitiveSelectandFieldcomponents.Why?
This provides a complete, labeled select field API similar to
InputControl, while keeping the lower-level select primitives available for custom composition.How?
SelectControlwith integrated label, description, details, and visually hidden label support.SelectControl.Itemso custom item children inherit the control size.SelectControlfrom the form package entrypoint.Testing Instructions
npm run test:unit packages/ui/src/form/select-control/test/index.test.tsx.npm run lint:js packages/ui/src/form/select-control packages/ui/src/form/index.ts.Design System/Components/Form/SelectControl.Screenshots or screencast