-
Notifications
You must be signed in to change notification settings - Fork 4.6k
Dataforms: Add icons to email and telephone controls #71514
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
Conversation
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
If we agree with this solution I may create a new util component under the controls folder used by both email and telephone controls to make having icons easier. |
| ); | ||
|
|
||
| return ( | ||
| <ValidatedText |
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.
This validatedtext component was added recently to share logic among text, email, and telephone. Can we instead add a prefix in ValidatedText and update it for email and telephone?
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.
Yeah, probably it would be better to update ValidatedText to use ValidatedInputControl instead of ValidatedTextControl. I'm not sure though if there are any nuances doing so.
In general it seems that InputControl is aimed to replace TextControl. I'll cc @ciampo for any input.
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.
Even if text doesn't need a prefix right now (and so it could still use ValidatedText), that's something we want to open to consumers soon (see) — I've seen a few use cases already where people are using custom Edits just for the prefix.
|
Flaky tests detected in e1b5412. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/17548430172
|
cdf50c0 to
96a4efa
Compare
96a4efa to
30fcb83
Compare
|
@jorgefilipecosta #71518 just landed adding support for a new |
1cbaf24 to
e137445
Compare
Hi @oandregal it is done 👍 |
…ndividual controls Changed approach from modifying individual form controls to enhancing the shared ValidatedText utility: - Replaced ValidatedTextControl with ValidatedInputControl for prefix support - Added automatic type-to-icon mapping (email → @, tel → mobile phone) - Added optional icon prop for manual overrides - Maintained backward compatibility while adding new capabilities This systematic approach benefits all text-based controls and provides better maintainability. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
Changed approach from ValidatedText automatically determining icons based on input type to individual controls explicitly passing their desired icons: - ValidatedText: Removed getIconForType function and automatic icon imports, now only uses explicitly passed icon prop - Email control: Added atSymbol import and explicitly passes icon prop - Telephone control: Added mobile import and explicitly passes icon prop This makes the icon choice more explicit and gives each control full control over its visual appearance. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
3ddfc71 to
00f95ac
Compare
| </InputControlPrefixWrapper> | ||
| ) : undefined | ||
| } | ||
| __next40pxDefaultSize |
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.
ValidatedInputControl has already this prop. I think we don't need it here.
| /** | ||
| * Optional icon to display as prefix. | ||
| */ | ||
| icon?: React.ComponentType | React.ReactElement; |
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.
I'm a bit late here, but wouldn't it be better if the prop is prefix and keep the flexibility of the inner component?
Why restrict to icon?
* Add @ icon prefix to email control with proper spacing * Refactor icon support by enhancing ValidatedText utility instead of individual controls Changed approach from modifying individual form controls to enhancing the shared ValidatedText utility: - Replaced ValidatedTextControl with ValidatedInputControl for prefix support - Added automatic type-to-icon mapping (email → @, tel → mobile phone) - Added optional icon prop for manual overrides - Maintained backward compatibility while adding new capabilities This systematic approach benefits all text-based controls and provides better maintainability. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> * Refactor to explicit icon passing instead of automatic type-based icons Changed approach from ValidatedText automatically determining icons based on input type to individual controls explicitly passing their desired icons: - ValidatedText: Removed getIconForType function and automatic icon imports, now only uses explicitly passed icon prop - Email control: Added atSymbol import and explicitly passes icon prop - Telephone control: Added mobile import and explicitly passes icon prop This makes the icon choice more explicit and gives each control full control over its visual appearance. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> * remove unneed var * add icon for url * Fix unit test --------- Co-authored-by: Claude <[email protected]>
Enhances the email control in DataViews with a visual @ symbol icon prefix, making email fields instantly recognizable.
Changes
ValidatedTextutility with customValidatedInputControlimplementationatSymbolicon from@wordpress/iconswith properInputControlPrefixWrapperspacingValidatedInputControlto components private API exportsTechnical Implementation
Email Control Enhancement
ValidatedTextControltoValidatedInputControlfor better prefix supportcustomValidityInputControlPrefixWrapperwithvariant="icon"for optimal spacing and alignmentIcon Integration
atSymbolicon from WordPress icons libraryIconcomponent for consistent renderingTest Plan