Skip to content
Open
Show file tree
Hide file tree
Changes from 8 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,11 @@ export default function ArrayControl< Item >( {
onChange,
hideLabelFromVision,
validity,
config,
}: DataFormControlProps< Item > ) {
const { label, placeholder, getValue, setValue, isValid } = field;
const value = getValue( { item: data } );
const { disabled = false } = config || {};

const { elements, isLoading } = useElements( {
elements: field.elements,
Expand Down Expand Up @@ -71,6 +73,7 @@ export default function ArrayControl< Item >( {
onChange={ onChangeControl }
placeholder={ placeholder }
suggestions={ elements?.map( ( element ) => element.value ) }
disabled={ disabled }
__experimentalValidateInput={ ( token: string ) => {
// If elements validation is required, check if token is valid
if ( field.isValid?.elements && elements ) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,10 @@ export default function Checkbox< Item >( {
data,
hideLabelFromVision,
validity,
config,
}: DataFormControlProps< Item > ) {
const { getValue, setValue, label, description, isValid } = field;
const { disabled = false } = config || {};

const onChangeControl = useCallback( () => {
onChange(
Expand All @@ -37,6 +39,7 @@ export default function Checkbox< Item >( {
help={ description }
checked={ getValue( { item: data } ) }
onChange={ onChangeControl }
disabled={ disabled }
/>
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,11 @@ const { ValidatedInputControl, Picker } = unlock( privateApis );
const ColorPicker = ( {
color,
onColorChange,
disabled,
}: {
color: string;
onColorChange: ( colorObject: any ) => void;
disabled?: boolean;
} ) => {
const validColor = color && colord( color ).isValid() ? color : '#ffffff';

Expand All @@ -38,20 +40,22 @@ const ColorPicker = ( {
<button
type="button"
onClick={ onToggle }
disabled={ disabled }
style={ {
width: '24px',
height: '24px',
borderRadius: '50%',
backgroundColor: validColor,
border: '1px solid #ddd',
cursor: 'pointer',
cursor: disabled ? 'default' : 'pointer',
outline: isOpen ? '2px solid #007cba' : 'none',
outlineOffset: '2px',
display: 'flex',
alignItems: 'center',
justifyContent: 'center',
padding: 0,
margin: 0,
opacity: disabled ? 0.4 : 1,
} }
aria-label="Open color picker"
/>
Expand All @@ -76,8 +80,10 @@ export default function Color< Item >( {
onChange,
hideLabelFromVision,
validity,
config,
}: DataFormControlProps< Item > ) {
const { label, placeholder, description, setValue, isValid } = field;
const { disabled = false } = config || {};
const value = field.getValue( { item: data } ) || '';

const handleColorChange = useCallback(
Expand Down Expand Up @@ -105,10 +111,12 @@ export default function Color< Item >( {
onChange={ handleInputChange }
hideLabelFromVision={ hideLabelFromVision }
type="text"
disabled={ disabled }
prefix={
<ColorPicker
color={ value }
onColorChange={ handleColorChange }
disabled={ disabled }
/>
}
/>
Expand Down
21 changes: 19 additions & 2 deletions packages/dataviews/src/components/dataform-controls/date.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,7 @@ function CalendarDateControl< Item >( {
onChange,
hideLabelFromVision,
validity,
config,
}: DataFormControlProps< Item > ) {
const {
id,
Expand All @@ -265,6 +266,7 @@ function CalendarDateControl< Item >( {
isValid,
format: fieldFormat,
} = field;
const { disabled = false } = config || {};
const [ selectedPresetId, setSelectedPresetId ] = useState< string | null >(
null
);
Expand Down Expand Up @@ -368,6 +370,8 @@ function CalendarDateControl< Item >( {
variant="tertiary"
isPressed={ isSelected }
size="small"
disabled={ disabled }
accessibleWhenDisabled={ false }
onClick={ () =>
handlePresetClick( preset )
}
Expand All @@ -381,7 +385,7 @@ function CalendarDateControl< Item >( {
variant="tertiary"
isPressed={ ! selectedPresetId }
size="small"
disabled={ !! selectedPresetId }
disabled={ !! selectedPresetId || disabled }
accessibleWhenDisabled={ false }
>
{ __( 'Custom' ) }
Expand All @@ -398,6 +402,7 @@ function CalendarDateControl< Item >( {
value={ value }
onChange={ handleManualDateChange }
required={ !! field.isValid?.required }
disabled={ disabled }
/>

{ /* Calendar widget */ }
Expand All @@ -411,6 +416,7 @@ function CalendarDateControl< Item >( {
onMonthChange={ setCalendarMonth }
timeZone={ timezoneString || undefined }
weekStartsOn={ weekStartsOn }
disabled={ disabled }
/>
</Stack>
</BaseControl>
Expand All @@ -424,8 +430,10 @@ function CalendarDateRangeControl< Item >( {
onChange,
hideLabelFromVision,
validity,
config,
}: DataFormControlProps< Item > ) {
const { id, label, getValue, setValue, format: fieldFormat } = field;
const { disabled = false } = config || {};
let value: DateRange;
const fieldValue = getValue( { item: data } );
if (
Expand Down Expand Up @@ -576,6 +584,8 @@ function CalendarDateRangeControl< Item >( {
variant="tertiary"
isPressed={ isSelected }
size="small"
disabled={ disabled }
accessibleWhenDisabled={ false }
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was very surprised by this value for accessibleWhenDisabled. I think this should always be set to true. I noticed that we have the same prop set to false (in trunk) too in a couple of places in DataViews package.

@jorgefilipecosta it seems all 3 occurrences are from you. Was that intentional for some reason?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yah it was intentional, if the custom button is disabled I don't see any value in including in on the tab sequence/ screen readers tree, it seems better to not have accessible, it does not adds any value.
On the range control we also disable the reset button packages/components/src/range-control/index.tsx if it is disable we are not very consistent on using accessibleWhenDisabled=false, there are other cases where we could use it but given that we almost never use it I can also understand removing it in this case.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm.. For date control I think:

  1. We use button in a non-interactive element which is already kind of wrong semantically for HTML. Additionally screen readers announce it as a button, but it doesn't do anything.
  2. We should either use another element (span, div, etc..) styles as we want, although maybe we could remove it entirely, since not having selected a preset indicates we are in 'custom', no?

For this PR, maybe it's similar to the range-control example you shared.

onClick={ () =>
handlePresetClick( preset )
}
Expand All @@ -590,7 +600,7 @@ function CalendarDateRangeControl< Item >( {
isPressed={ ! selectedPresetId }
size="small"
accessibleWhenDisabled={ false }
disabled={ !! selectedPresetId }
disabled={ !! selectedPresetId || disabled }
>
{ __( 'Custom' ) }
</Button>
Expand All @@ -614,6 +624,7 @@ function CalendarDateRangeControl< Item >( {
handleManualDateChange( 'from', newValue )
}
required={ !! field.isValid?.required }
disabled={ disabled }
/>
<InputControl
__next40pxDefaultSize
Expand All @@ -626,6 +637,7 @@ function CalendarDateRangeControl< Item >( {
handleManualDateChange( 'to', newValue )
}
required={ !! field.isValid?.required }
disabled={ disabled }
/>
</Stack>

Expand All @@ -637,6 +649,7 @@ function CalendarDateRangeControl< Item >( {
onMonthChange={ setCalendarMonth }
timeZone={ timezone.string || undefined }
weekStartsOn={ weekStartsOn }
disabled={ disabled }
/>
</Stack>
</BaseControl>
Expand All @@ -651,6 +664,7 @@ export default function DateControl< Item >( {
hideLabelFromVision,
operator,
validity,
config,
}: DataFormControlProps< Item > ) {
if ( operator === OPERATOR_IN_THE_PAST || operator === OPERATOR_OVER ) {
return (
Expand All @@ -661,6 +675,7 @@ export default function DateControl< Item >( {
onChange={ onChange }
hideLabelFromVision={ hideLabelFromVision }
operator={ operator }
config={ config }
/>
);
}
Expand All @@ -673,6 +688,7 @@ export default function DateControl< Item >( {
onChange={ onChange }
hideLabelFromVision={ hideLabelFromVision }
validity={ validity }
config={ config }
/>
);
}
Expand All @@ -684,6 +700,7 @@ export default function DateControl< Item >( {
onChange={ onChange }
hideLabelFromVision={ hideLabelFromVision }
validity={ validity }
config={ config }
/>
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,10 @@ function CalendarDateTimeControl< Item >( {
onChange,
hideLabelFromVision,
validity,
config,
}: DataFormControlProps< Item > ) {
const { id, label, description, setValue, getValue, isValid } = field;
const { disabled = false } = config || {};
const fieldValue = getValue( { item: data } );
const value = typeof fieldValue === 'string' ? fieldValue : undefined;

Expand Down Expand Up @@ -179,6 +181,7 @@ function CalendarDateTimeControl< Item >( {
onMonthChange={ setCalendarMonth }
timeZone={ timezoneString || undefined }
weekStartsOn={ weekStartsOn }
disabled={ disabled }
/>
{ /* Manual datetime input */ }
<ValidatedInputControl
Expand All @@ -197,6 +200,7 @@ function CalendarDateTimeControl< Item >( {
: ''
}
onChange={ handleManualDateTimeChange }
disabled={ disabled }
/>
</Stack>
</BaseControl>
Expand All @@ -210,6 +214,7 @@ export default function DateTime< Item >( {
hideLabelFromVision,
operator,
validity,
config,
}: DataFormControlProps< Item > ) {
if ( operator === OPERATOR_IN_THE_PAST || operator === OPERATOR_OVER ) {
return (
Expand All @@ -220,6 +225,7 @@ export default function DateTime< Item >( {
onChange={ onChange }
hideLabelFromVision={ hideLabelFromVision }
operator={ operator }
config={ config }
/>
);
}
Expand All @@ -231,6 +237,7 @@ export default function DateTime< Item >( {
onChange={ onChange }
hideLabelFromVision={ hideLabelFromVision }
validity={ validity }
config={ config }
/>
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ export default function Email< Item >( {
onChange,
hideLabelFromVision,
validity,
config,
}: DataFormControlProps< Item > ) {
const { disabled = false } = config || {};
return (
<ValidatedText
{ ...{
Expand All @@ -28,6 +30,7 @@ export default function Email< Item >( {
onChange,
hideLabelFromVision,
validity,
disabled,
type: 'email',
prefix: (
<InputControlPrefixWrapper variant="icon">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,5 @@ import type { DataFormControlProps } from '../../types';
import ValidatedNumber from './utils/validated-number';

export default function Number< Item >( props: DataFormControlProps< Item > ) {
return <ValidatedNumber { ...props } />;
return <ValidatedNumber { ...props } config={ props.config } />;
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,10 @@ export default function Password< Item >( {
onChange,
hideLabelFromVision,
validity,
config,
}: DataFormControlProps< Item > ) {
const [ isVisible, setIsVisible ] = useState( false );
const { disabled = false } = config || {};

const toggleVisibility = useCallback( () => {
setIsVisible( ( prev ) => ! prev );
Expand All @@ -35,6 +37,7 @@ export default function Password< Item >( {
onChange,
hideLabelFromVision,
validity,
disabled,
type: isVisible ? 'text' : 'password',
suffix: (
<InputControlSuffixWrapper variant="control">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,10 @@ export default function Radio< Item >( {
onChange,
hideLabelFromVision,
validity,
config,
}: DataFormControlProps< Item > ) {
const { label, description, getValue, setValue, isValid } = field;
const { disabled = false } = config || {};
const { elements, isLoading } = useElements( {
elements: field.elements,
getElements: field.getElements,
Expand All @@ -48,6 +50,7 @@ export default function Radio< Item >( {
options={ elements }
selected={ value }
hideLabelFromVision={ hideLabelFromVision }
disabled={ disabled }
/>
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,10 @@ export default function Select< Item >( {
onChange,
hideLabelFromVision,
validity,
config,
}: DataFormControlProps< Item > ) {
const { type, label, description, getValue, setValue, isValid } = field;
const { disabled = false } = config || {};

const isMultiple = type === 'array';
const value = getValue( { item: data } ) ?? ( isMultiple ? [] : '' );
Expand Down Expand Up @@ -53,6 +55,7 @@ export default function Select< Item >( {
__next40pxDefaultSize
hideLabelFromVision={ hideLabelFromVision }
multiple={ isMultiple }
disabled={ disabled }
/>
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ export default function Telephone< Item >( {
onChange,
hideLabelFromVision,
validity,
config,
}: DataFormControlProps< Item > ) {
const { disabled = false } = config || {};
return (
<ValidatedText
{ ...{
Expand All @@ -28,6 +30,7 @@ export default function Telephone< Item >( {
onChange,
hideLabelFromVision,
validity,
disabled,
type: 'tel',
prefix: (
<InputControlPrefixWrapper variant="icon">
Expand Down
Loading
Loading