Skip to content

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.

Sign up for GitHub

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

Merged
jorgefilipecosta merged 6 commits into from
Sep 8, 2025
Merged

Dataforms: Add icons to email and telephone controls #71514

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
adf041f
Add @ icon prefix to email control with proper spacing
jorgefilipecosta Sep 5, 2025
13c21cd
Refactor icon support by enhancing ValidatedText utility instead of i…
jorgefilipecosta Sep 5, 2025
b8c1c16
Refactor to explicit icon passing instead of automatic type-based icons
jorgefilipecosta Sep 5, 2025
5ede93d
remove unneed var
jorgefilipecosta Sep 5, 2025
00f95ac
add icon for url
jorgefilipecosta Sep 5, 2025
e1b5412
Fix unit test
jorgefilipecosta Sep 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions packages/components/src/private-apis.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import Badge from './badge';
import { DateCalendar, DateRangeCalendar, TZDate } from './calendar';
import {
ValidatedCheckboxControl,
ValidatedInputControl,
ValidatedNumberControl,
ValidatedTextControl,
ValidatedToggleControl,
Expand All @@ -33,6 +34,7 @@ lock( privateApis, {
DateCalendar,
DateRangeCalendar,
TZDate,
ValidatedInputControl,
ValidatedCheckboxControl,
ValidatedNumberControl,
ValidatedTextControl,
Expand Down
14 changes: 13 additions & 1 deletion packages/dataviews/src/dataform-controls/email.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
/**
* WordPress dependencies
*/
import { atSymbol } from '@wordpress/icons';

/**
* Internal dependencies
*/
Expand All @@ -12,7 +17,14 @@ export default function Email< Item >( {
}: DataFormControlProps< Item > ) {
return (
<ValidatedText
Copy link
Member

@oandregal oandregal Sep 5, 2025

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?

Copy link
Contributor

@ntsekouras ntsekouras Sep 5, 2025

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.

Copy link
Member

@oandregal oandregal Sep 5, 2025

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.

{ ...{ data, field, onChange, hideLabelFromVision, type: 'email' } }
{ ...{
data,
field,
onChange,
hideLabelFromVision,
type: 'email',
icon: atSymbol,
} }
/>
);
}
14 changes: 13 additions & 1 deletion packages/dataviews/src/dataform-controls/telephone.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
/**
* WordPress dependencies
*/
import { mobile } from '@wordpress/icons';

/**
* Internal dependencies
*/
Expand All @@ -12,7 +17,14 @@ export default function Telephone< Item >( {
}: DataFormControlProps< Item > ) {
return (
<ValidatedText
{ ...{ data, field, onChange, hideLabelFromVision, type: 'tel' } }
{ ...{
data,
field,
onChange,
hideLabelFromVision,
type: 'tel',
icon: mobile,
} }
/>
);
}
14 changes: 13 additions & 1 deletion packages/dataviews/src/dataform-controls/url.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
/**
* WordPress dependencies
*/
import { link } from '@wordpress/icons';

/**
* Internal dependencies
*/
Expand All @@ -12,7 +17,14 @@ export default function Url< Item >( {
}: DataFormControlProps< Item > ) {
return (
<ValidatedText
{ ...{ data, field, onChange, hideLabelFromVision, type: 'url' } }
{ ...{
data,
field,
onChange,
hideLabelFromVision,
type: 'url',
icon: link,
} }
/>
);
}
25 changes: 21 additions & 4 deletions packages/dataviews/src/dataform-controls/utils/validated-text.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
/**
* WordPress dependencies
*/
import { privateApis } from '@wordpress/components';
import {
Icon,
privateApis,
__experimentalInputControlPrefixWrapper as InputControlPrefixWrapper,
} from '@wordpress/components';
import { useCallback, useState } from '@wordpress/element';

/**
Expand All @@ -10,14 +14,18 @@ import { useCallback, useState } from '@wordpress/element';
import type { DataFormControlProps } from '../../types';
import { unlock } from '../../lock-unlock';

const { ValidatedTextControl } = unlock( privateApis );
const { ValidatedInputControl } = unlock( privateApis );

export type DataFormValidatedTextControlProps< Item > =
DataFormControlProps< Item > & {
/**
* The input type of the control.
*/
type?: 'text' | 'email' | 'tel' | 'url';
/**
* Optional icon to display as prefix.
*/
icon?: React.ComponentType | React.ReactElement;
Copy link
Contributor

@ntsekouras ntsekouras Sep 9, 2025

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?

};

export default function ValidatedText< Item >( {
Expand All @@ -26,13 +34,14 @@ export default function ValidatedText< Item >( {
onChange,
hideLabelFromVision,
type,
icon,
}: DataFormValidatedTextControlProps< Item > ) {
const { id, label, placeholder, description } = field;
const value = field.getValue( { item: data } );
const [ customValidity, setCustomValidity ] =
useState<
React.ComponentProps<
typeof ValidatedTextControl
typeof ValidatedInputControl
>[ 'customValidity' ]
>( undefined );

Expand All @@ -45,7 +54,7 @@ export default function ValidatedText< Item >( {
);

return (
<ValidatedTextControl
<ValidatedInputControl
required={ !! field.isValid?.required }
onValidate={ ( newValue: any ) => {
const message = field.isValid?.custom?.(
Expand Down Expand Up @@ -74,6 +83,14 @@ export default function ValidatedText< Item >( {
onChange={ onChangeControl }
hideLabelFromVision={ hideLabelFromVision }
type={ type }
prefix={
icon ? (
<InputControlPrefixWrapper variant="icon">
<Icon icon={ icon } />
</InputControlPrefixWrapper>
) : undefined
}
__next40pxDefaultSize
Copy link
Contributor

@ntsekouras ntsekouras Sep 9, 2025

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.

/>
);
}
4 changes: 2 additions & 2 deletions packages/dataviews/src/test/dataform.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ describe( 'DataForm component', () => {
expect( onChange ).toHaveBeenCalledTimes( newValue.length );
for ( let i = 0; i < newValue.length; i++ ) {
expect( onChange ).toHaveBeenNthCalledWith( i + 1, {
title: newValue[ i ],
title: newValue.slice( 0, i + 1 ),
} );
}
} );
Expand Down Expand Up @@ -408,7 +408,7 @@ describe( 'DataForm component', () => {
expect( onChange ).toHaveBeenCalledTimes( newValue.length );
for ( let i = 0; i < newValue.length; i++ ) {
expect( onChange ).toHaveBeenNthCalledWith( i + 1, {
title: newValue[ i ],
title: newValue.slice( 0, i + 1 ),
} );
}
} );
Expand Down
Loading