-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: input in card becomes invisible on hover and making date-input and time-input consistent #4394
Conversation
… time-input consistent
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
🦋 Changeset detectedLatest commit: 6a81dfc The changes in this PR will be included in the next version bump. This PR includes changesets to release 12 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 |
WalkthroughThis pull request focuses on enhancing the styling and consistency of input-related components in the Changes
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
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.changeset/late-doors-rhyme.md (1)
5-5
: Improve the changelog message clarity and grammar.Consider revising the message for better clarity:
-Fixing the input in card to not become invisible on hover and making time-input and date-input consistent to other inputs. +Fix input visibility in cards on hover and ensure consistency between time-input, date-input, and other input components.🧰 Tools
🪛 LanguageTool
[grammar] ~5-~5: Before the countable noun ‘on’ an article or a possessive pronoun is necessary.
Context: ...nput in card to not become invisible on hover and making time-input and date-input co...(IN_NN_CC_VBG)
[uncategorized] ~5-~5: The preposition “with” seems more likely in this position.
Context: ...ng time-input and date-input consistent to other inputs.(AI_EN_LECTOR_REPLACEMENT_PREPOSITION)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.changeset/late-doors-rhyme.md
(1 hunks)packages/core/theme/src/components/date-input.ts
(9 hunks)packages/core/theme/src/components/input.ts
(1 hunks)packages/core/theme/src/components/select.ts
(3 hunks)
🧰 Additional context used
📓 Learnings (1)
packages/core/theme/src/components/select.ts (2)
Learnt from: macci001
PR: nextui-org/nextui#3881
File: packages/core/theme/src/components/select.ts:227-231
Timestamp: 2024-11-12T02:48:07.324Z
Learning: In `packages/core/theme/src/components/select.ts`, changes may sometimes be formatting-only. Ensure to verify whether changes are only formatting-related before flagging potential issues.
Learnt from: macci001
PR: nextui-org/nextui#3881
File: packages/core/theme/src/components/select.ts:255-255
Timestamp: 2024-11-12T02:48:07.324Z
Learning: When updating the hover state for the faded variant of the select component, changing the background from `bg-secondary-50` to `bg-secondary-200` aligns with making the styling consistent with the input component.
🪛 LanguageTool
.changeset/late-doors-rhyme.md
[grammar] ~5-~5: Before the countable noun ‘on’ an article or a possessive pronoun is necessary.
Context: ...nput in card to not become invisible on hover and making time-input and date-input co...
(IN_NN_CC_VBG)
[uncategorized] ~5-~5: The preposition “with” seems more likely in this position.
Context: ...ng time-input and date-input consistent to other inputs.
(AI_EN_LECTOR_REPLACEMENT_PREPOSITION)
🔇 Additional comments (7)
packages/core/theme/src/components/date-input.ts (2)
230-230
: LGTM! Background color updates improve visibility.
The changes to background colors in the flat variant for all color schemes (primary, secondary, success, warning, danger) follow a consistent pattern and improve visibility.
Also applies to: 241-241, 252-252, 263-263, 274-274
280-345
: LGTM! New faded variant styles maintain consistency.
The addition of faded variant compound styles for all color schemes maintains consistency with other input components while providing proper visual feedback through border colors.
packages/core/theme/src/components/select.ts (3)
41-42
: LGTM! Improved hover and focus states.
The update to use bg-default-200
for hover and focus states improves visibility and aligns with other input components.
93-107
: LGTM! Consistent color variants for selector icon.
The addition of color variants for the selector icon enhances visual consistency across different color schemes.
235-235
: LGTM! Consistent hover state in compound variants.
The hover state update in compound variants maintains consistency with the base variant changes.
packages/core/theme/src/components/input.ts (2)
79-79
: Improved hover state visibility
The change from bg-default-50
to bg-default-200
provides better contrast, making the input visible on hover while maintaining a logical progression with the focus state (bg-default-100
).
Line range hint 1-1
: Type exports enhance developer experience
The addition of InputVariantProps
and InputSlots
types improves type safety and makes it easier for developers to work with the input component's variants and slots.
Also applies to: 1-1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Closes #4371
📝 Description
This PR:
⛳️ Current behavior (updates)
🚀 New behavior
Screen.Recording.2024-12-18.at.4.04.36.PM.mov
Screen.Recording.2024-12-18.at.4.05.19.PM.mov
Screen.Recording.2024-12-18.at.4.08.25.PM.mov
Screen.Recording.2024-12-18.at.4.07.39.PM.mov
Screen.Recording.2024-12-18.at.4.09.48.PM.mov
Screen.Recording.2024-12-18.at.4.11.23.PM.mov
Screen.Recording.2024-12-18.at.4.14.03.PM.mov
Screen.Recording.2024-12-18.at.4.12.56.PM.mov
💣 Is this a breaking change (Yes/No): No
Summary by CodeRabbit
New Features
Bug Fixes
Documentation