feat: add label/value object support for Select options, streamlines normalization #573
Conversation
🦋 Changeset detectedLatest commit: 5011336 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 5011336:
|
Co-authored-by: gsimone <[email protected]>
Co-authored-by: gsimone <[email protected]>
Co-authored-by: gsimone <[email protected]>
| export const ValueLabelObjectsWithFunctions = () => { | ||
| const fn1 = () => console.log('Function 1') | ||
| const fn2 = () => console.log('Function 2') | ||
| const fn3 = () => console.log('Function 3') | ||
|
|
||
| const values = useControls({ | ||
| myFunction: { | ||
| options: [ | ||
| { value: fn1, label: 'First Function' }, | ||
| { value: fn2, label: 'Second Function' }, | ||
| { value: fn3, label: 'Third Function' }, | ||
| ], | ||
| }, | ||
| }) | ||
|
|
||
| return ( | ||
| <div> | ||
| <pre>Selected: {values.myFunction.name || 'anonymous function'}</pre> | ||
| <button onClick={values.myFunction}>Call selected function</button> | ||
| </div> | ||
| ) | ||
| } |
There was a problem hiding this comment.
Removed the ValueLabelObjectsWithFunctions story in commit 5579ffe. The ValueLabelObjects story remains to demonstrate the new API.
|
@copilot Fix the comments and add a story for the new api |
Co-authored-by: gsimone <[email protected]>
gsimone
changed the title
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/leva/stories/inputs/Select.stories.tsx (1)
60-67: Clarify whether deprecated story should be retained.The comment marks this as "Unsupported/deprecated use case," yet the story remains. Consider either:
- Removing the story if the functionality is truly deprecated
- Clarifying in the comment why it's retained (e.g., "kept for backward compatibility testing")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (10)
.changeset/fair-donkeys-cheer.md(1 hunks)package.json(1 hunks)packages/leva/package.json(1 hunks)packages/leva/src/plugins/Select/select-plugin.test.ts(1 hunks)packages/leva/src/plugins/Select/select-plugin.ts(2 hunks)packages/leva/src/plugins/Select/select-types.ts(1 hunks)packages/leva/src/types/public.test.ts(1 hunks)packages/leva/src/types/public.ts(1 hunks)packages/leva/stories/advanced/Busy.stories.tsx(1 hunks)packages/leva/stories/inputs/Select.stories.tsx(2 hunks)
🧰 Additional context used
🪛 Biome (2.1.2)
packages/leva/src/types/public.test.ts
[error] 66-66: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
🔇 Additional comments (18)
package.json (1)
108-109: LGTM: Test tooling added appropriately.The addition of vitest supports the new test suite for the Select plugin.
packages/leva/package.json (1)
35-35: LGTM: Zod dependency added for new validation approach.The zod dependency supports the new schema-based validation in the Select plugin, aligning with Zod 4 features.
.changeset/fair-donkeys-cheer.md (1)
1-5: LGTM: Changeset properly documents the new feature.The patch release designation is appropriate for a backward-compatible feature addition.
packages/leva/stories/advanced/Busy.stories.tsx (1)
58-58: Verify the array-to-string conversion is intentional.The select options changed from
['x', 'y', ['x', 'y']]to['x', 'y', 'x,y'], converting the nested array value to a string. Ensure this doesn't break code expecting the third option to be an array value['x', 'y']rather than the string'x,y'.packages/leva/src/types/public.test.ts (1)
65-74: LGTM: Type test correctly validates the new SelectOption API.The type test appropriately verifies that
useControlswith value/label object options yields the expected type. The static analysis warning about conditional hook usage is a false positive—this is a type test file usingexpectType, not runtime code with conditional hook calls.packages/leva/stories/inputs/Select.stories.tsx (4)
25-27: LGTM: Documentation comments improve story clarity.The added comments clearly explain each story's purpose and the expected behavior.
Also applies to: 34-44
37-40: LGTM: NoValue story demonstrates useful default behavior.This story appropriately shows that when no value is provided, the first option becomes the default.
69-90: LGTM: Component refactor improves clarity.Renaming to
ComponentA/ComponentBand rendering the selected component directly is cleaner than the previous implementation.
92-103: LGTM: ValueLabelObjects story effectively demonstrates the new API.This story clearly shows the label/value object format with human-readable labels ("red", "green", "blue") and technical values (hex codes), which is the core feature of this PR.
packages/leva/src/plugins/Select/select-plugin.test.ts (4)
4-282: Excellent comprehensive test coverage for normalize function.The tests thoroughly cover all three option formats (primitives, objects, value/label arrays) with various value types, edge cases, and backward compatibility scenarios. The use of console.warn spies to verify warning behavior is appropriate.
284-331: Schema validation tests are thorough.The tests verify that all three valid option formats are accepted while properly rejecting invalid inputs. The test at lines 310-315 correctly validates that mixed invalid arrays are rejected at the schema level.
333-344: LGTM!The sanitize tests appropriately verify both the success case (value exists) and error case (value not found).
346-361: LGTM!The format tests cover all relevant cases: finding a value at various positions and handling the not-found scenario.
packages/leva/src/plugins/Select/select-plugin.ts (5)
38-38: The previous review comment is incorrect—allUsecasesIS being used.The
allUsecasesschema is used at line 51 withinselectInputSchemato validate theoptionsfield. The variable is not unused.
2-55: Well-structured zod-based validation with clear documentation.The schema definitions are well-organized and documented with examples. The three-format approach (primitives, key/value objects, value/label objects) correctly validates all supported input types. The optional label field at line 33 properly supports partial label specification.
66-106: Robust format detection with appropriate fallback.The normalize function correctly handles all three option formats in the right order of specificity. The value/label object format is checked first (most specific), followed by primitives, then key/value objects. The fallback at lines 93-104 provides helpful diagnostic output for invalid formats.
108-126: Value selection logic correctly handles edge cases with backward compatibility.The logic appropriately handles missing values (defaults to first option), explicitly undefined values (passes through without warning), and values not in options (warns and returns undefined). The condition at line 120 (
value !== undefined) ensures that explicitly undefined values don't trigger warnings, which is reasonable for backward compatibility even though it's subtle.
57-64: LGTM!The
sanitizeandformatfunctions are straightforward and correct. The explicitreturn valueinsanitizeat line 59 properly returns the validated value.
…-for-options-bindings
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
packages/leva/stories/inputs/Select.stories.tsx (1)
16-26: Consider tighter typing for the Render component.The
args: anyparameter weakens type safety. Consider using a more specific type or a generic parameter to better capture the expected shape of story args.Example:
-const Render = (args: any) => { +const Render = <T extends Record<string, any>>(args: T) => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
packages/leva/package.json(1 hunks)packages/leva/src/plugins/Select/select-plugin.ts(2 hunks)packages/leva/src/plugins/Select/select-types.ts(1 hunks)packages/leva/src/types/public.test.ts(1 hunks)packages/leva/src/types/public.ts(2 hunks)packages/leva/stories/inputs/Select.stories.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/leva/package.json
🧰 Additional context used
🪛 Biome (2.1.2)
packages/leva/src/types/public.test.ts
[error] 64-64: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
🪛 GitHub Check: Build, lint, and test
packages/leva/src/plugins/Select/select-plugin.ts
[warning] 125-125:
Unexpected console statement
[warning] 98-98:
Unexpected console statement
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: chromatic
🔇 Additional comments (2)
packages/leva/stories/inputs/Select.stories.tsx (2)
2-2: Modern Storybook pattern looks good.The migration to
satisfies MetaandStoryObj<typeof meta>follows current Storybook best practices for type-safe story definitions.Also applies to: 8-14
100-113: Excellent demonstration of the new value/label object API.This story clearly showcases the new feature: hex color codes as technical values paired with human-readable labels. The example effectively demonstrates the PR's objective to decouple displayed labels from returned values.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai generate docstrings |
✅ Actions performedInitiated docstring generation; will generate only if new commits exist. |
|
Warning Docstrings generation - IN PROGRESS Generating docstrings for this pull request |
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/leva/src/plugins/Select/select-plugin.ts(2 hunks)packages/leva/src/plugins/Select/select-types.ts(1 hunks)packages/leva/src/types/public.ts(2 hunks)packages/leva/stories/inputs/Select.stories.tsx(1 hunks)
🧰 Additional context used
🪛 GitHub Check: Build, lint, and test
packages/leva/src/plugins/Select/select-plugin.ts
[warning] 130-130:
Unexpected console statement
[warning] 103-103:
Unexpected console statement
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/leva/src/plugins/Select/select-plugin.ts (1)
82-142: Robust normalization with comprehensive format detection.The rewritten
normalizefunction correctly handles all three option formats with proper fallbacks and developer feedback. The sequential schema checks and gathered key/value extraction are well-structured.Minor considerations:
Empty options edge case: If
optionsis an empty array (which passes schema validation), line 127 setsvalue = gatheredValues[0]which would beundefined. This is likely acceptable behavior, but consider whether the schema should enforce non-empty arrays using.nonempty()if this is undesirable.Console warnings: Lines 110 and 137 use
console.warnfor developer feedback. While this is appropriate for a UI library to communicate misuse, consider using a logging abstraction if one exists in the codebase for consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/leva/src/plugins/Select/select-plugin.ts(2 hunks)packages/leva/stories/inputs/Select.stories.tsx(1 hunks)
🧰 Additional context used
🪛 GitHub Check: Build, lint, and test
packages/leva/src/plugins/Select/select-plugin.ts
[warning] 137-137:
Unexpected console statement
[warning] 110-110:
Unexpected console statement
🔇 Additional comments (4)
packages/leva/src/plugins/Select/select-plugin.ts (3)
4-11: Excellent fix for function identity preservation.The switch from
z.function()toz.custom()with a type guard correctly addresses the previous critical issue where function values were being wrapped in zod proxies. This preserves the original function references, ensuring theFunctionAsOptionsstory works correctly.
13-68: Well-structured schema definitions with clear documentation.The progressive schema definitions (primitives → objects → value/label objects → union) are logically organized and include helpful inline examples. The three supported formats are clearly documented and the type exports provide a clean public API.
71-71: Clean migration to zod-based validation.The schema export now leverages
selectInputSchema.safeParse(s).success, providing consistent validation with the rest of the codebase.packages/leva/stories/inputs/Select.stories.tsx (1)
20-280: Excellent test coverage across all option formats.The five stories comprehensively demonstrate and test all supported Select formats:
- Simple: Array of primitives where values serve as labels
- NoValue: Default first-option selection behavior
- CustomLabels: Object format with custom key-label/value pairs
- FunctionAsOptions: Function values preserved correctly (validates the
z.customfix)- ValueLabelObjects: New
{ value, label }format with hex colors and readable labelsEach story includes thorough
playfunctions that validate initial state, user interactions, and post-interaction state. This provides robust regression protection and clearly demonstrates the API for developers.
✅ Actions performedInitiated docstring generation; will generate only if new commits exist. |
Enables human-readable labels for Select option values, particularly useful when values are hex codes or other technical representations.
Changes
Core logic (
select-plugin.ts):{value, label}objects by checking first element{value, label}objectsType definitions:
SelectOption<T>type toselect-types.tsandpublic.tsSelectSettingsto acceptSelectOption<U>[]Examples (
Select.stories.tsx):ValueLabelObjectsstory demonstrating hex colors with friendly namesUsage
Backward compatible with existing array and object formats.
Demo
Labels display in the dropdown while hex color values are returned.
optionsbindings? #551Original prompt
optionsbindings? #551💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.
Summary by CodeRabbit