Skip to content
Open
Show file tree
Hide file tree
Changes from 4 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
9 changes: 5 additions & 4 deletions packages/block-editor/src/hooks/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@ import { useDispatch, useRegistry } from '@wordpress/data';
import { createHigherOrderComponent } from '@wordpress/compose';
import { addFilter } from '@wordpress/hooks';

/**
* External dependencies
*/
import clsx from 'clsx';

/**
* Internal dependencies
*/
Expand All @@ -20,10 +25,6 @@ import { useSettingsForBlockElement } from '../components/global-styles/hooks';
import { getValueFromObjectPath, setImmutably } from '../utils/object';
import { store as blockEditorStore } from '../store';
import { unlock } from '../lock-unlock';
/**
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.

Is this change necessary? Perhaps this PR needs a rebase from trunk?

* External dependencies
*/
import clsx from 'clsx';

/**
* Removed falsy values from nested object.
Expand Down
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 }
/>
);
}
Loading
Loading