fix(number-input): include labelPlacement in dependency array#5866
Merged
Conversation
…HeroUIProvider context
🦋 Changeset detectedLatest commit: b595ea7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
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 |
|
@KumJungMin is attempting to deploy a commit to the HeroUI Inc Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
WalkthroughAdds a changeset and tests, and updates the NumberInput hook to read labelPlacement from HeroUIProvider and thread it into variant/slot generation so label placement can be inherited from context while still allowing explicit prop override. Changes
Sequence Diagram(s)sequenceDiagram
participant App as App
participant Provider as HeroUIProvider
participant NumberInput as NumberInput
participant Hook as useNumberInput
participant Variant as variant(numberInput)
App->>Provider: Render with labelPlacement="outside"
Provider->>NumberInput: provide context
NumberInput->>Hook: call useNumberInput(props)
activate Hook
Hook->>Hook: read labelPlacement from context or props
Hook->>Variant: pass labelPlacement into variant props
Variant->>Hook: return slot props/classes reflecting placement
Hook-->>NumberInput: return slots
deactivate Hook
NumberInput->>App: render with label positioned "outside"
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
⏰ 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). (4)
🔇 Additional comments (1)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/components/number-input/src/use-number-input.ts (1)
232-249: hasStartContent in dependency array is likely redundant and can be removed.
hasStartContentis used for rendering logic (data attributes and label placement) but is not a variant key for thenumberInputtheme function. Since the variant function ignores this property, the memoizedslotsresult is identical regardless ofhasStartContent's value. Including it in the dependency array causes unnecessary recomputation without changing the output.Suggested fix: Remove
hasStartContentfrom the dependency array at line 246.[ objectToDeps(variantProps), isInvalid, labelPlacement, isClearable, disableAnimation, ]
🧹 Nitpick comments (1)
packages/components/number-input/__tests__/number-input.test.tsx (1)
685-711: Good test coverage for the provider inheritance behavior.The test suite properly verifies:
- NumberInput inherits
labelPlacementfrom HeroUIProvider context- Explicit
labelPlacementprop overrides provider contextBoth scenarios are essential for validating the fix.
Minor note on test brittleness: The tests rely on CSS class pattern matching (
/translate-y.*100%/) to verify label placement. While practical for styling components, this approach is brittle and may break if theme implementation changes. Consider documenting this dependency or, if feasible, testing via other means (e.g., computed styles or data attributes).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.changeset/brave-laws-enjoy.md(1 hunks)packages/components/number-input/__tests__/number-input.test.tsx(2 hunks)packages/components/number-input/src/use-number-input.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
packages/components/number-input/__tests__/number-input.test.tsx (1)
packages/components/number-input/src/index.ts (1)
NumberInput(10-10)
packages/components/number-input/src/use-number-input.ts (1)
packages/utilities/shared-utils/src/common/functions.ts (1)
objectToDeps(149-159)
⏰ 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). (5)
- GitHub Check: Continuous Release
- GitHub Check: Prettier
- GitHub Check: TypeScript
- GitHub Check: ESLint
- GitHub Check: Build
🔇 Additional comments (1)
.changeset/brave-laws-enjoy.md (1)
1-5: LGTM! Changeset correctly documents the patch.The changeset appropriately categorizes this as a patch-level change (bug fix/enhancement without breaking changes) and clearly describes the new capability.
@heroui/accordion
@heroui/alert
@heroui/autocomplete
@heroui/avatar
@heroui/badge
@heroui/breadcrumbs
@heroui/button
@heroui/calendar
@heroui/card
@heroui/checkbox
@heroui/chip
@heroui/code
@heroui/date-input
@heroui/date-picker
@heroui/divider
@heroui/drawer
@heroui/dropdown
@heroui/form
@heroui/image
@heroui/input
@heroui/input-otp
@heroui/kbd
@heroui/link
@heroui/listbox
@heroui/menu
@heroui/modal
@heroui/navbar
@heroui/number-input
@heroui/pagination
@heroui/popover
@heroui/progress
@heroui/radio
@heroui/ripple
@heroui/scroll-shadow
@heroui/select
@heroui/skeleton
@heroui/slider
@heroui/snippet
@heroui/spacer
@heroui/spinner
@heroui/switch
@heroui/table
@heroui/tabs
@heroui/toast
@heroui/tooltip
@heroui/user
@heroui/react
@heroui/system
@heroui/system-rsc
@heroui/theme
@heroui/use-aria-accordion
@heroui/use-aria-accordion-item
@heroui/use-aria-button
@heroui/use-aria-link
@heroui/use-aria-modal-overlay
@heroui/use-aria-multiselect
@heroui/use-aria-overlay
@heroui/use-callback-ref
@heroui/use-clipboard
@heroui/use-data-scroll-overflow
@heroui/use-disclosure
@heroui/use-draggable
@heroui/use-form-reset
@heroui/use-image
@heroui/use-infinite-scroll
@heroui/use-intersection-observer
@heroui/use-is-mobile
@heroui/use-is-mounted
@heroui/use-measure
@heroui/use-pagination
@heroui/use-real-shape
@heroui/use-ref-state
@heroui/use-resize
@heroui/use-safe-layout-effect
@heroui/use-scroll-position
@heroui/use-ssr
@heroui/use-theme
@heroui/use-update-effect
@heroui/use-viewport-size
@heroui/aria-utils
@heroui/dom-animation
@heroui/framer-utils
@heroui/react-rsc-utils
@heroui/react-utils
@heroui/shared-icons
@heroui/shared-utils
@heroui/stories-utils
@heroui/test-utils
commit: |
This was referenced Oct 27, 2025
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.
Closes #5845
📝 Description
labelPlacement(e.g., "outside") defined in HeroUIProvider was not applied to NumberInput.Problem:
The label was expected to be placed outside, but it was rendered inside instead.
labelPlacementis properly passed into the hook and slot calculation so the component correctly reflects the global configuration.Solved:
The label is now correctly rendered outside as expected.
⛳️ Current behavior (updates)
labelPlacementvalue from HeroUIProvider.🚀 New behavior
labelPlacementvalue from HeroUIProvider.labelPlacementon NumberInput can still override the global value if specified.💣 Is this a breaking change (Yes/No):
No
📝 Additional Information
numberInput()style generator and dependency array in useMemo, ensuring reactive updates when the global provider changes.Summary by CodeRabbit
New Features
Tests
Chores