Skip to content
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

Jump to bottom

Dataform bulk editing support #67344

Open
wants to merge 10 commits into
base: trunk
Choose a base branch
from
+166 −58
Open

Dataform bulk editing support #67344

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
0444bed
Support mixed render for bulk editing
louwie17 Nov 26, 2024
3e5e62c
Allow array of items being passed into form for bulk editing
louwie17 Nov 26, 2024
7869b30
Add translation for Mixed label
louwie17 Nov 27, 2024
97ae2b5
Default supportsBulk to false
louwie17 Nov 27, 2024
489623a
Remove field layout supportsBulk config
louwie17 Nov 27, 2024
fdc7bf8
Default bulk support to true, unless custom Edit is used
louwie17 Nov 27, 2024
4df0165
Update supports bulk setting name
louwie17 Dec 2, 2024
ac03ed1
Address renaming missed in the last commit
louwie17 Dec 2, 2024
c27bd65
Add mixed symbol value for bulk editing
louwie17 Dec 3, 2024
366883a
Skip underscore keys and support objects in bulk editing values
louwie17 Dec 3, 2024
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
54 changes: 52 additions & 2 deletions packages/dataviews/src/components/dataform/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,44 @@ import type { DataFormProps } from '../../types';
import { DataFormProvider } from '../dataform-context';
import { normalizeFields } from '../../normalize-fields';
import { DataFormLayout } from '../../dataforms-layouts/data-form-layout';
import { MIXED_VALUE } from '../../constants';

export default function DataForm< Item >( {
/**
* Loops through the list of data items and returns an object with the intersecting ( same ) key and values.
* Skips keys that start with an underscore.
*
* @param data list of items.
*/
function getIntersectingValues< Item extends object >( data: Item[] ): Item {
Copy link
Contributor

Choose a reason for hiding this comment

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

I just noticed that this function is trying to combine the items into an Item object. Unfortunately, I don't think that's possible really because the Symbol is not a valid Item value and the nesting is not the right thing to do.

I think what we should be doing here is calling field.getValue for all the fields (maybe the fields that are within the form) and compute the result in a merged object (that is not Item) who keys are the field keys and values are the result of getValue calls.

This is necessary because we can't really assume/know the shape of valid Item. this is something we discussed IRL with @oandregal and @gigitux at Rome core days.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm yeah, I can give this approach a try. I agree it makes more sense, but will need quite a bit more changes I believe.
For example, if we do generate this new merged object using getValue and receive an array of data do we pass the array of data down to each field? or the merged object? or just the first item in the array ( probably the whole array, but currently a lot of fields pass that straight into getValue ).

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree it's not very straightforward. Maybe the current PR can be a good interim solution (although not entirely "correct"). Should we try it in a separate PR to see how they compare?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should we try it in a separate PR to see how they compare?

Yeah a different PR is a good idea, otherwise it may be hard to see the pros/cons of each approach.

const intersectingValues = {} as Item;
const keys = Object.keys( data[ 0 ] ) as Array< keyof Item >;
for ( const key of keys ) {
// Skip keys that start with underscore.
if ( key.toString().startsWith( '_' ) ) {
continue;
}
const [ firstRecord, ...remainingRecords ] = data;

if ( typeof firstRecord[ key ] === 'object' ) {
// Traverse through nested objects.
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you think we should traverse nested objects?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Some fields may rely on the nested values only, incase we want to support that for bulk editing. Although we don't have direct use case for that in core right now, aside from maybe metadata fields.
But if we go with your suggestion below, this won't be necessary as we can rely on the passed in fields only.

intersectingValues[ key ] = getIntersectingValues(
data.map( ( item ) => item[ key ] as object )
) as Item[ keyof Item ];
} else {
const intersects = remainingRecords.every( ( item ) => {
return item[ key ] === firstRecord[ key ];
} );
if ( intersects ) {
intersectingValues[ key ] = firstRecord[ key ];
} else {
intersectingValues[ key ] = MIXED_VALUE as Item[ keyof Item ];
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is an exact replicate of this comment: #67344 (comment) I am not sure if you meant to do that or if GH messed up?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, forget about this one :P

Copy link
Contributor

Choose a reason for hiding this comment

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

This is actually where I had my initial "Symbol" comment :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Aaaha that makes a lot of sense! :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added in: c27bd65

}
return intersectingValues;
}

export default function DataForm< Item extends object >( {
data,
form,
fields,
Expand All @@ -22,13 +58,27 @@ export default function DataForm< Item >( {
[ fields ]
);

const flattenedData = useMemo( () => {
if ( Array.isArray( data ) ) {
return getIntersectingValues< Item >( data );
}
return data;
}, [ data ] );

if ( ! form.fields ) {
return null;
}

const isBulkEditing = Array.isArray( data );

return (
<DataFormProvider fields={ normalizedFields }>
<DataFormLayout data={ data } form={ form } onChange={ onChange } />
<DataFormLayout
data={ flattenedData }
form={ form }
onChange={ onChange }
isBulkEditing={ isBulkEditing }
/>
</DataFormProvider>
);
}
3 changes: 3 additions & 0 deletions packages/dataviews/src/constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,3 +68,6 @@ export const sortIcons = {
export const LAYOUT_TABLE = 'table';
export const LAYOUT_GRID = 'grid';
export const LAYOUT_LIST = 'list';

// Dataform mixed value.
export const MIXED_VALUE = Symbol.for( 'DATAFORM_MIXED_VALUE' );
39 changes: 37 additions & 2 deletions packages/dataviews/src/dataforms-layouts/data-form-layout.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,37 @@ import { useContext, useMemo } from '@wordpress/element';
/**
* Internal dependencies
*/
import type { Form, FormField, SimpleFormField } from '../types';
import type {
CombinedFormField,
Form,
FormField,
NormalizedField,
SimpleFormField,
} from '../types';
import { getFormFieldLayout } from './index';
import DataFormContext from '../components/dataform-context';
import { isCombinedField } from './is-combined-field';
import normalizeFormFields from '../normalize-form-fields';

export function DataFormLayout< Item >( {
function doesCombinedFieldSupportBulkEdits< Item >(
combinedField: CombinedFormField,
fieldDefinitions: NormalizedField< Item >[]
): boolean {
return combinedField.children.some( ( child ) => {
const fieldId = typeof child === 'string' ? child : child.id;

return fieldDefinitions.find(
( fieldDefinition ) => fieldDefinition.id === fieldId
)?.supportsBulkEditing;
} );
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Checks if 'any' of the nested fields support bulk, returns 'true' in that case.


export function DataFormLayout< Item extends object >( {
data,
form,
onChange,
children,
isBulkEditing,
}: {
data: Item;
form: Form;
Expand All @@ -31,6 +51,7 @@ export function DataFormLayout< Item >( {
} ) => React.JSX.Element | null,
field: FormField
) => React.JSX.Element;
isBulkEditing?: boolean;
} ) {
const { fields: fieldDefinitions } = useContext( DataFormContext );

Expand Down Expand Up @@ -69,6 +90,19 @@ export function DataFormLayout< Item >( {
return null;
}

if (
isBulkEditing &&
( ( isCombinedField( formField ) &&
! doesCombinedFieldSupportBulkEdits(
formField,
fieldDefinitions
) ) ||
( fieldDefinition &&
! fieldDefinition.supportsBulkEditing ) )
) {
return null;
}

if ( children ) {
return children( FieldLayout, formField );
}
Expand All @@ -79,6 +113,7 @@ export function DataFormLayout< Item >( {
data={ data }
field={ formField }
onChange={ onChange }
isBulkEditing={ isBulkEditing }
/>
);
} ) }
Expand Down
19 changes: 17 additions & 2 deletions packages/dataviews/src/dataforms-layouts/panel/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import type {
import DataFormContext from '../../components/dataform-context';
import { DataFormLayout } from '../data-form-layout';
import { isCombinedField } from '../is-combined-field';
import { MIXED_VALUE } from '../../constants';

function DropdownHeader( {
title,
Expand Down Expand Up @@ -59,20 +60,22 @@ function DropdownHeader( {
);
}

function PanelDropdown< Item >( {
function PanelDropdown< Item extends object >( {
fieldDefinition,
popoverAnchor,
labelPosition = 'side',
data,
onChange,
field,
isBulkEditing,
}: {
fieldDefinition: NormalizedField< Item >;
popoverAnchor: HTMLElement | null;
labelPosition: 'side' | 'top' | 'none';
data: Item;
onChange: ( value: any ) => void;
field: FormField;
isBulkEditing?: boolean;
} ) {
const fieldLabel = isCombinedField( field )
? field.label
Expand Down Expand Up @@ -111,6 +114,9 @@ function PanelDropdown< Item >( {
[ popoverAnchor ]
);

const fieldValue = fieldDefinition.getValue( { item: data } );
const showMixedValue = isBulkEditing && fieldValue === MIXED_VALUE;

return (
<Dropdown
contentClassName="dataforms-layouts-panel__field-dropdown"
Expand Down Expand Up @@ -138,7 +144,11 @@ function PanelDropdown< Item >( {
) }
onClick={ onToggle }
>
<fieldDefinition.render item={ data } />
{ showMixedValue ? (
__( 'Mixed' )
) : (
<fieldDefinition.render item={ data } />
) }
</Button>
) }
renderContent={ ( { onClose } ) => (
Expand All @@ -148,6 +158,7 @@ function PanelDropdown< Item >( {
data={ data }
form={ form as Form }
onChange={ onChange }
isBulkEditing={ isBulkEditing }
>
{ ( FieldLayout, nestedField ) => (
<FieldLayout
Expand All @@ -171,6 +182,7 @@ export default function FormPanelField< Item >( {
data,
field,
onChange,
isBulkEditing,
}: FieldLayoutProps< Item > ) {
const { fields } = useContext( DataFormContext );
const fieldDefinition = fields.find( ( fieldDef ) => {
Expand Down Expand Up @@ -221,6 +233,7 @@ export default function FormPanelField< Item >( {
data={ data }
onChange={ onChange }
labelPosition={ labelPosition }
isBulkEditing={ isBulkEditing }
/>
</div>
</VStack>
Expand All @@ -237,6 +250,7 @@ export default function FormPanelField< Item >( {
data={ data }
onChange={ onChange }
labelPosition={ labelPosition }
isBulkEditing={ isBulkEditing }
/>
</div>
);
Expand All @@ -259,6 +273,7 @@ export default function FormPanelField< Item >( {
data={ data }
onChange={ onChange }
labelPosition={ labelPosition }
isBulkEditing={ isBulkEditing }
/>
</div>
</HStack>
Expand Down
2 changes: 1 addition & 1 deletion packages/dataviews/src/dataforms-layouts/regular/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ function Header( { title }: { title: string } ) {
);
}

export default function FormRegularField< Item >( {
export default function FormRegularField< Item extends object >( {
data,
field,
onChange,
Expand Down
1 change: 1 addition & 0 deletions packages/dataviews/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@ export { VIEW_LAYOUTS } from './dataviews-layouts';
export { filterSortAndPaginate } from './filter-and-sort-data-view';
export type * from './types';
export { isItemValid } from './validation';
export { MIXED_VALUE as DATAFORM_MIXED_VALUE } from './constants';
7 changes: 7 additions & 0 deletions packages/dataviews/src/normalize-fields.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,12 @@ export function normalizeFields< Item >(
);
};

let supportsBulkEditing = true;
// If custom Edit component is passed in we default to false for bulk edit support.
if ( typeof field.Edit === 'function' || field.supportsBulkEditing ) {
supportsBulkEditing = field.supportsBulkEditing ?? false;
}

const render =
field.render || ( field.elements ? renderFromElements : getValue );

Expand All @@ -76,6 +82,7 @@ export function normalizeFields< Item >(
sort,
isValid,
Edit,
supportsBulkEditing,
enableHiding: field.enableHiding ?? true,
enableSorting: field.enableSorting ?? true,
};
Expand Down
8 changes: 7 additions & 1 deletion packages/dataviews/src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,11 @@ export type Field< Item > = {
* Defaults to `item[ field.id ]`.
*/
getValue?: ( args: { item: Item } ) => any;

/**
* Whether the field supports bulk editing.
*/
supportsBulkEditing?: boolean;
};

export type NormalizedField< Item > = Field< Item > & {
Expand Down Expand Up @@ -564,7 +569,7 @@ export type Form = {
};

export interface DataFormProps< Item > {
data: Item;
data: Item | Item[];
fields: Field< Item >[];
form: Form;
onChange: ( value: Record< string, any > ) => void;
Expand All @@ -575,4 +580,5 @@ export interface FieldLayoutProps< Item > {
field: FormField;
onChange: ( value: any ) => void;
hideLabelFromVision?: boolean;
isBulkEditing?: boolean;
}
Loading
Loading