-
Notifications
You must be signed in to change notification settings - Fork 4
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
[WEB-4082] Include example for input field prefix usage, rename poorly named icon #544
Conversation
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe changes introduce a new input field variation in the Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
🪧 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
CodeRabbit Configuration File (
|
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: 2
🧹 Outside diff range and nitpick comments (2)
src/core/styles/Forms.stories.tsx (2)
85-95
: Make the currency prefix implementation more flexibleThe current implementation has hardcoded dimensions and positioning which might not work well with different font sizes or icon sizes. Consider making it more flexible and reusable.
- <div className="relative"> - <div className="h-32 w-32 absolute left-8 top-8 flex items-center justify-center"> - $ - </div> - <input - type="search" - className="ui-input pl-40" - placeholder="With icon" - autoComplete="off" - /> - </div> + <div className="relative"> + <div className="absolute left-2 top-1/2 -translate-y-1/2 flex items-center justify-center pointer-events-none"> + <Icon name="icon-gui-currency-dollar" size="1rem" /> + </div> + <input + type="search" + className="ui-input pl-8" + style={{ paddingLeft: 'calc(2rem + 0.5em)' }} + placeholder="With icon" + autoComplete="off" + /> + </div>
83-96
: Consider creating a reusable InputWithPrefix componentSince this is a common pattern that might be used across different forms, consider extracting it into a reusable component that can handle different types of prefixes (not just currency).
Example implementation:
interface InputWithPrefixProps extends React.InputHTMLAttributes<HTMLInputElement> { prefix: React.ReactNode; prefixAriaLabel: string; } const InputWithPrefix: React.FC<InputWithPrefixProps> = ({ prefix, prefixAriaLabel, className, ...props }) => ( <div className="relative"> <span id={`${props.id}-prefix`} className="sr-only">{prefixAriaLabel}</span> <div className="absolute left-2 top-1/2 -translate-y-1/2 flex items-center justify-center pointer-events-none"> {prefix} </div> <input {...props} className={`ui-input ${className}`} style={{ paddingLeft: 'calc(2rem + 0.5em)' }} aria-describedby={`${props.id}-prefix`} /> </div> );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (3)
src/core/Icon/__snapshots__/Icon.stories.tsx.snap
is excluded by!**/*.snap
src/core/icons/icon-display-connection-state-recovery.svg
is excluded by!**/*.svg
src/core/styles/__snapshots__/Forms.stories.tsx.snap
is excluded by!**/*.snap
📒 Files selected for processing (1)
src/core/styles/Forms.stories.tsx
(1 hunks)
6997313
to
1732e0a
Compare
@matt423 The other PR linked in the description has all the CSS icon class generation stuff I mentioned this morning, but that has proven somewhat problematic to get working - as such, I've closed that and this PR just has the part that may be helpful to you in the form of an example of how to recreate what's requested in that design. Best way to see this is in Storybook under the "with character insert" entry in |
Relates to #543 but removes the parts where the icon CSS classes are generated.
Summary by CodeRabbit