fix(input): label placement with file input type #4366
Conversation
🦋 Changeset detectedLatest commit: d873d4f The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 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 |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis pull request addresses a bug in the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/components/input/src/use-input.ts (1)
Line range hint
1-489: Consider adding test cases for file input scenarios.The implementation looks solid and properly fixes the file input label placement issue. To ensure long-term maintainability and prevent regressions, consider adding test cases that cover:
- File input with different label placements
- File input interaction with data attributes
- File input styling consistency
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.changeset/chilly-rules-marry.md(1 hunks)packages/components/input/src/use-input.ts(1 hunks)
🔇 Additional comments (5)
.changeset/chilly-rules-marry.md (1)
1-5: LGTM! Changeset correctly configured for the bug fix.
The changeset properly identifies this as a patch update for the input component, which is appropriate for a bug fix addressing label placement.
packages/components/input/src/use-input.ts (4)
16-16: LGTM! Simplified imports by removing unused warn utility.
The removal of the warn import aligns with the removal of the file input type warning message.
Line range hint 182-189: LGTM! Correct handling of file input props.
The special handling for file input type is well-implemented. Removing the value and onChange props is crucial as they would otherwise block the first attempt of file upload with an empty string value.
Line range hint 314-329: LGTM! Proper handling of data attributes for file inputs.
File inputs are now correctly handled in the data attributes by being considered as "filled", which ensures proper styling and visual consistency.
Line range hint 279-285: Verify the impact on existing file input implementations.
The label placement logic has been simplified to treat file inputs like other input types. While this is a good change, we should ensure it doesn't affect existing implementations.
✅ Verification successful
File input handling remains consistent and properly implemented
The code shows that file inputs are still handled appropriately with special considerations:
- The component correctly removes
valueandonChangeprops for file inputs to prevent blocking file uploads - The stories demonstrate both single and multiple file input usage without any label placement issues
- There's no evidence of any breaking changes in the label placement logic for file inputs
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for file input usage with label placement
rg -l 'type="file".*labelPlacement' || rg -l 'labelPlacement.*type="file"'
Length of output: 75
Script:
#!/bin/bash
# Let's try a broader search for file input usage in the codebase
rg -l 'type="file"'
# Also search for any test files that might be testing file input behavior
rg -l 'file.*input' '**/*test*' '**/*spec*'
# Search for any documentation or examples of file input usage
fd -g '*docs*.{tsx,ts,mdx,md}' -x rg -l 'file.*input' {}
Length of output: 323
Script:
#!/bin/bash
# Let's examine the actual implementation and usage in the found files
rg -A 5 -B 5 'type="file"' packages/components/input/src/use-input.ts packages/components/input/stories/input.stories.tsx
# Check if there were any previous special cases for file inputs
git log -p packages/components/input/src/use-input.ts | rg -A 5 -B 5 'file.*input'
Length of output: 2211
Closes #4364
📝 Description
Not sure what was the reason why
insideis not supported. Probably this logic was added before changingdata-filled&data-filled-within.⛳️ Current behavior (updates)
When using
labelPlacement=inside, it throwsInput with file type doesn't support inside label. Converting to outside ...🚀 New behavior
💣 Is this a breaking change (Yes/No):
📝 Additional Information
Summary by CodeRabbit
New Features
Bug Fixes
Documentation