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.

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

Add bulk editing support by passing array down to the fields #67584

Open
wants to merge 14 commits into
base: trunk
Choose a base branch
from

Conversation

louwie17
Copy link
Contributor

@louwie17 louwie17 commented Dec 4, 2024

What?

Allows consumers to pass in an array of items to be edited. The panel view will show "Mixed" when the values are different across the items.

Why?

Partially addresses: #65685
The header for bulk editing will be addressed in a separate PR.

How?

This is an alternative solution to: #67344

DataForm allows an array of items to be passed in.
When an array of items is passed in it does a couple things:

  • The data prop now supports Item and Item[]
  • The render method receives a value prop which can be the Mixed symbol.
  • The data form layouts call getValue and pass the value down. They also set it to Mixed if bulk editing and the value isn't the same across all records.

Some questions/follow up:

  • The render method takes a single item prop, how do we handle this when passing in an array of items? Should we replace this with the data and value props similar to the edit method? Or add support for an items prop?

Testing Instructions

  1. Enable the "Quick Edit in DataViews" experiment
  2. Enable the 2024 or 2025 theme and go to Appearance > Editor > Pages
  3. Change the view in the top right to Table and open the right side bar
  4. Select multiple pages, it should show the title, status, author, and date fields
  5. The fields should show Mixed if the values are different across pages.
  6. Update the status for example, it should remove Mixed and show the selected status.

Testing Instructions for Keyboard

Screenshots or screencast

bulk-editing.mp4

@louwie17 louwie17 added [Type] Enhancement A suggestion for improvement. [Feature] DataViews Work surrounding upgrading and evolving views in the site editor and beyond labels Dec 4, 2024
data: Item;
export type DataFormControlProps<
Item,
SupportsBulkEditing extends boolean = true,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added this additional type to make it easier to handle within fields that don't support bulk editing, but maybe there is a better way?

Copy link
Contributor

Choose a reason for hiding this comment

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

It is just a personal preference, but I would prefer type composition:

export type DataFormControlProps< Item > = {
	data: Item;
	field: NormalizedField< Item >;
	onChange: ( value: Record< string, any > ) => void;
	hideLabelFromVision?: boolean;
	value: any;
};

export type WithBulkEditing< Item > = {
	data: Item | Item[];
};

And use in this way:

export const ParentEdit = ( {
	data,
	field,
	onChange,
}: DataFormControlProps< BasePost > & WithBulkEditing< BasePost > ) => {

We could do export already the composed type:

export type DataFormControlWithBulkEditingProps< Item > = DataFormControlProps< Item > & WithBulkEditing< Item >;

export const ParentEdit = ( {
	data,
	field,
	onChange,
}: DataFormControlWithBulkEditingProps< BasePost > 

@@ -27,7 +27,7 @@ const SlugEdit = ( {
field,
onChange,
data,
}: DataFormControlProps< BasePost > ) => {
}: DataFormControlProps< BasePost, false > ) => {
const { id } = field;

const slug = field.getValue( { item: data } ) || getSlug( data );
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If slug where to support bulk edits how would we handle the getSlug call here.
field.getValue( { item: data } ) can be replaced by the value prop.

Copy link

github-actions bot commented Dec 4, 2024

Size Change: +565 B (+0.03%)

Total Size: 1.84 MB

Filename Size Change
build/edit-site/index.min.js 221 kB +213 B (+0.1%)
build/editor/index.min.js 115 kB +352 B (+0.31%)
ℹ️ View Unchanged
Filename Size
build-module/a11y/index.min.js 482 B
build-module/block-library/file/view.min.js 447 B
build-module/block-library/form/view.min.js 533 B
build-module/block-library/image/view.min.js 1.77 kB
build-module/block-library/navigation/view.min.js 1.16 kB
build-module/block-library/query/view.min.js 742 B
build-module/block-library/search/view.min.js 616 B
build-module/interactivity-router/index.min.js 3.04 kB
build-module/interactivity/debug.min.js 17.2 kB
build-module/interactivity/index.min.js 13.6 kB
build/a11y/index.min.js 952 B
build/annotations/index.min.js 2.26 kB
build/api-fetch/index.min.js 2.32 kB
build/autop/index.min.js 2.12 kB
build/blob/index.min.js 579 B
build/block-directory/index.min.js 7.26 kB
build/block-directory/style-rtl.css 1 kB
build/block-directory/style.css 1 kB
build/block-editor/content-rtl.css 4.47 kB
build/block-editor/content.css 4.46 kB
build/block-editor/default-editor-styles-rtl.css 394 B
build/block-editor/default-editor-styles.css 394 B
build/block-editor/index.min.js 259 kB
build/block-editor/style-rtl.css 15.7 kB
build/block-editor/style.css 15.7 kB
build/block-library/blocks/archives/editor-rtl.css 84 B
build/block-library/blocks/archives/editor.css 83 B
build/block-library/blocks/archives/style-rtl.css 90 B
build/block-library/blocks/archives/style.css 90 B
build/block-library/blocks/audio/editor-rtl.css 149 B
build/block-library/blocks/audio/editor.css 151 B
build/block-library/blocks/audio/style-rtl.css 132 B
build/block-library/blocks/audio/style.css 132 B
build/block-library/blocks/audio/theme-rtl.css 134 B
build/block-library/blocks/audio/theme.css 134 B
build/block-library/blocks/avatar/editor-rtl.css 115 B
build/block-library/blocks/avatar/editor.css 115 B
build/block-library/blocks/avatar/style-rtl.css 104 B
build/block-library/blocks/avatar/style.css 104 B
build/block-library/blocks/button/editor-rtl.css 265 B
build/block-library/blocks/button/editor.css 265 B
build/block-library/blocks/button/style-rtl.css 555 B
build/block-library/blocks/button/style.css 555 B
build/block-library/blocks/buttons/editor-rtl.css 291 B
build/block-library/blocks/buttons/editor.css 291 B
build/block-library/blocks/buttons/style-rtl.css 345 B
build/block-library/blocks/buttons/style.css 345 B
build/block-library/blocks/calendar/style-rtl.css 240 B
build/block-library/blocks/calendar/style.css 240 B
build/block-library/blocks/categories/editor-rtl.css 132 B
build/block-library/blocks/categories/editor.css 131 B
build/block-library/blocks/categories/style-rtl.css 152 B
build/block-library/blocks/categories/style.css 152 B
build/block-library/blocks/code/editor-rtl.css 53 B
build/block-library/blocks/code/editor.css 53 B
build/block-library/blocks/code/style-rtl.css 139 B
build/block-library/blocks/code/style.css 139 B
build/block-library/blocks/code/theme-rtl.css 122 B
build/block-library/blocks/code/theme.css 122 B
build/block-library/blocks/columns/editor-rtl.css 108 B
build/block-library/blocks/columns/editor.css 108 B
build/block-library/blocks/columns/style-rtl.css 420 B
build/block-library/blocks/columns/style.css 420 B
build/block-library/blocks/comment-author-avatar/editor-rtl.css 124 B
build/block-library/blocks/comment-author-avatar/editor.css 124 B
build/block-library/blocks/comment-author-name/style-rtl.css 72 B
build/block-library/blocks/comment-author-name/style.css 72 B
build/block-library/blocks/comment-content/style-rtl.css 120 B
build/block-library/blocks/comment-content/style.css 120 B
build/block-library/blocks/comment-date/style-rtl.css 65 B
build/block-library/blocks/comment-date/style.css 65 B
build/block-library/blocks/comment-edit-link/style-rtl.css 70 B
build/block-library/blocks/comment-edit-link/style.css 70 B
build/block-library/blocks/comment-reply-link/style-rtl.css 71 B
build/block-library/blocks/comment-reply-link/style.css 71 B
build/block-library/blocks/comment-template/style-rtl.css 200 B
build/block-library/blocks/comment-template/style.css 199 B
build/block-library/blocks/comments-pagination-numbers/editor-rtl.css 122 B
build/block-library/blocks/comments-pagination-numbers/editor.css 121 B
build/block-library/blocks/comments-pagination/editor-rtl.css 238 B
build/block-library/blocks/comments-pagination/editor.css 231 B
build/block-library/blocks/comments-pagination/style-rtl.css 245 B
build/block-library/blocks/comments-pagination/style.css 241 B
build/block-library/blocks/comments-title/editor-rtl.css 75 B
build/block-library/blocks/comments-title/editor.css 75 B
build/block-library/blocks/comments/editor-rtl.css 842 B
build/block-library/blocks/comments/editor.css 842 B
build/block-library/blocks/comments/style-rtl.css 637 B
build/block-library/blocks/comments/style.css 637 B
build/block-library/blocks/cover/editor-rtl.css 631 B
build/block-library/blocks/cover/editor.css 631 B
build/block-library/blocks/cover/style-rtl.css 1.7 kB
build/block-library/blocks/cover/style.css 1.69 kB
build/block-library/blocks/details/editor-rtl.css 65 B
build/block-library/blocks/details/editor.css 65 B
build/block-library/blocks/details/style-rtl.css 86 B
build/block-library/blocks/details/style.css 86 B
build/block-library/blocks/embed/editor-rtl.css 331 B
build/block-library/blocks/embed/editor.css 331 B
build/block-library/blocks/embed/style-rtl.css 419 B
build/block-library/blocks/embed/style.css 419 B
build/block-library/blocks/embed/theme-rtl.css 133 B
build/block-library/blocks/embed/theme.css 133 B
build/block-library/blocks/file/editor-rtl.css 326 B
build/block-library/blocks/file/editor.css 326 B
build/block-library/blocks/file/style-rtl.css 278 B
build/block-library/blocks/file/style.css 279 B
build/block-library/blocks/footnotes/style-rtl.css 198 B
build/block-library/blocks/footnotes/style.css 197 B
build/block-library/blocks/form-input/editor-rtl.css 229 B
build/block-library/blocks/form-input/editor.css 229 B
build/block-library/blocks/form-input/style-rtl.css 357 B
build/block-library/blocks/form-input/style.css 357 B
build/block-library/blocks/form-submission-notification/editor-rtl.css 344 B
build/block-library/blocks/form-submission-notification/editor.css 341 B
build/block-library/blocks/form-submit-button/style-rtl.css 69 B
build/block-library/blocks/form-submit-button/style.css 69 B
build/block-library/blocks/freeform/editor-rtl.css 2.6 kB
build/block-library/blocks/freeform/editor.css 2.6 kB
build/block-library/blocks/gallery/editor-rtl.css 946 B
build/block-library/blocks/gallery/editor.css 951 B
build/block-library/blocks/gallery/style-rtl.css 1.83 kB
build/block-library/blocks/gallery/style.css 1.82 kB
build/block-library/blocks/gallery/theme-rtl.css 108 B
build/block-library/blocks/gallery/theme.css 108 B
build/block-library/blocks/group/editor-rtl.css 334 B
build/block-library/blocks/group/editor.css 334 B
build/block-library/blocks/group/style-rtl.css 103 B
build/block-library/blocks/group/style.css 103 B
build/block-library/blocks/group/theme-rtl.css 79 B
build/block-library/blocks/group/theme.css 79 B
build/block-library/blocks/heading/style-rtl.css 188 B
build/block-library/blocks/heading/style.css 188 B
build/block-library/blocks/html/editor-rtl.css 346 B
build/block-library/blocks/html/editor.css 347 B
build/block-library/blocks/image/editor-rtl.css 799 B
build/block-library/blocks/image/editor.css 799 B
build/block-library/blocks/image/style-rtl.css 1.6 kB
build/block-library/blocks/image/style.css 1.59 kB
build/block-library/blocks/image/theme-rtl.css 137 B
build/block-library/blocks/image/theme.css 137 B
build/block-library/blocks/latest-comments/style-rtl.css 355 B
build/block-library/blocks/latest-comments/style.css 354 B
build/block-library/blocks/latest-posts/editor-rtl.css 139 B
build/block-library/blocks/latest-posts/editor.css 138 B
build/block-library/blocks/latest-posts/style-rtl.css 520 B
build/block-library/blocks/latest-posts/style.css 520 B
build/block-library/blocks/list/style-rtl.css 107 B
build/block-library/blocks/list/style.css 107 B
build/block-library/blocks/loginout/style-rtl.css 61 B
build/block-library/blocks/loginout/style.css 61 B
build/block-library/blocks/media-text/editor-rtl.css 321 B
build/block-library/blocks/media-text/editor.css 320 B
build/block-library/blocks/media-text/style-rtl.css 552 B
build/block-library/blocks/media-text/style.css 550 B
build/block-library/blocks/more/editor-rtl.css 427 B
build/block-library/blocks/more/editor.css 427 B
build/block-library/blocks/navigation-link/editor-rtl.css 644 B
build/block-library/blocks/navigation-link/editor.css 645 B
build/block-library/blocks/navigation-link/style-rtl.css 192 B
build/block-library/blocks/navigation-link/style.css 191 B
build/block-library/blocks/navigation-submenu/editor-rtl.css 295 B
build/block-library/blocks/navigation-submenu/editor.css 294 B
build/block-library/blocks/navigation/editor-rtl.css 2.2 kB
build/block-library/blocks/navigation/editor.css 2.2 kB
build/block-library/blocks/navigation/style-rtl.css 2.24 kB
build/block-library/blocks/navigation/style.css 2.23 kB
build/block-library/blocks/nextpage/editor-rtl.css 392 B
build/block-library/blocks/nextpage/editor.css 392 B
build/block-library/blocks/page-list/editor-rtl.css 378 B
build/block-library/blocks/page-list/editor.css 378 B
build/block-library/blocks/page-list/style-rtl.css 192 B
build/block-library/blocks/page-list/style.css 192 B
build/block-library/blocks/paragraph/editor-rtl.css 236 B
build/block-library/blocks/paragraph/editor.css 236 B
build/block-library/blocks/paragraph/style-rtl.css 341 B
build/block-library/blocks/paragraph/style.css 340 B
build/block-library/blocks/post-author-biography/style-rtl.css 74 B
build/block-library/blocks/post-author-biography/style.css 74 B
build/block-library/blocks/post-author-name/style-rtl.css 69 B
build/block-library/blocks/post-author-name/style.css 69 B
build/block-library/blocks/post-author/editor-rtl.css 107 B
build/block-library/blocks/post-author/editor.css 107 B
build/block-library/blocks/post-author/style-rtl.css 188 B
build/block-library/blocks/post-author/style.css 189 B
build/block-library/blocks/post-comments-form/editor-rtl.css 96 B
build/block-library/blocks/post-comments-form/editor.css 96 B
build/block-library/blocks/post-comments-form/style-rtl.css 527 B
build/block-library/blocks/post-comments-form/style.css 528 B
build/block-library/blocks/post-content/style-rtl.css 61 B
build/block-library/blocks/post-content/style.css 61 B
build/block-library/blocks/post-date/style-rtl.css 62 B
build/block-library/blocks/post-date/style.css 62 B
build/block-library/blocks/post-excerpt/editor-rtl.css 71 B
build/block-library/blocks/post-excerpt/editor.css 71 B
build/block-library/blocks/post-excerpt/style-rtl.css 155 B
build/block-library/blocks/post-excerpt/style.css 155 B
build/block-library/blocks/post-featured-image/editor-rtl.css 729 B
build/block-library/blocks/post-featured-image/editor.css 726 B
build/block-library/blocks/post-featured-image/style-rtl.css 347 B
build/block-library/blocks/post-featured-image/style.css 347 B
build/block-library/blocks/post-navigation-link/style-rtl.css 215 B
build/block-library/blocks/post-navigation-link/style.css 214 B
build/block-library/blocks/post-template/editor-rtl.css 99 B
build/block-library/blocks/post-template/editor.css 98 B
build/block-library/blocks/post-template/style-rtl.css 399 B
build/block-library/blocks/post-template/style.css 398 B
build/block-library/blocks/post-terms/style-rtl.css 96 B
build/block-library/blocks/post-terms/style.css 96 B
build/block-library/blocks/post-time-to-read/style-rtl.css 70 B
build/block-library/blocks/post-time-to-read/style.css 70 B
build/block-library/blocks/post-title/style-rtl.css 162 B
build/block-library/blocks/post-title/style.css 162 B
build/block-library/blocks/preformatted/style-rtl.css 125 B
build/block-library/blocks/preformatted/style.css 125 B
build/block-library/blocks/pullquote/editor-rtl.css 134 B
build/block-library/blocks/pullquote/editor.css 134 B
build/block-library/blocks/pullquote/style-rtl.css 351 B
build/block-library/blocks/pullquote/style.css 350 B
build/block-library/blocks/pullquote/theme-rtl.css 167 B
build/block-library/blocks/pullquote/theme.css 167 B
build/block-library/blocks/query-pagination-numbers/editor-rtl.css 121 B
build/block-library/blocks/query-pagination-numbers/editor.css 118 B
build/block-library/blocks/query-pagination/editor-rtl.css 154 B
build/block-library/blocks/query-pagination/editor.css 154 B
build/block-library/blocks/query-pagination/style-rtl.css 237 B
build/block-library/blocks/query-pagination/style.css 237 B
build/block-library/blocks/query-title/style-rtl.css 64 B
build/block-library/blocks/query-title/style.css 64 B
build/block-library/blocks/query/editor-rtl.css 527 B
build/block-library/blocks/query/editor.css 527 B
build/block-library/blocks/quote/style-rtl.css 238 B
build/block-library/blocks/quote/style.css 238 B
build/block-library/blocks/quote/theme-rtl.css 233 B
build/block-library/blocks/quote/theme.css 236 B
build/block-library/blocks/read-more/style-rtl.css 138 B
build/block-library/blocks/read-more/style.css 138 B
build/block-library/blocks/rss/editor-rtl.css 101 B
build/block-library/blocks/rss/editor.css 101 B
build/block-library/blocks/rss/style-rtl.css 288 B
build/block-library/blocks/rss/style.css 287 B
build/block-library/blocks/search/editor-rtl.css 199 B
build/block-library/blocks/search/editor.css 199 B
build/block-library/blocks/search/style-rtl.css 660 B
build/block-library/blocks/search/style.css 658 B
build/block-library/blocks/search/theme-rtl.css 113 B
build/block-library/blocks/search/theme.css 113 B
build/block-library/blocks/separator/editor-rtl.css 100 B
build/block-library/blocks/separator/editor.css 100 B
build/block-library/blocks/separator/style-rtl.css 248 B
build/block-library/blocks/separator/style.css 248 B
build/block-library/blocks/separator/theme-rtl.css 195 B
build/block-library/blocks/separator/theme.css 195 B
build/block-library/blocks/shortcode/editor-rtl.css 286 B
build/block-library/blocks/shortcode/editor.css 286 B
build/block-library/blocks/site-logo/editor-rtl.css 806 B
build/block-library/blocks/site-logo/editor.css 803 B
build/block-library/blocks/site-logo/style-rtl.css 218 B
build/block-library/blocks/site-logo/style.css 218 B
build/block-library/blocks/site-tagline/editor-rtl.css 87 B
build/block-library/blocks/site-tagline/editor.css 87 B
build/block-library/blocks/site-tagline/style-rtl.css 65 B
build/block-library/blocks/site-tagline/style.css 65 B
build/block-library/blocks/site-title/editor-rtl.css 85 B
build/block-library/blocks/site-title/editor.css 85 B
build/block-library/blocks/site-title/style-rtl.css 143 B
build/block-library/blocks/site-title/style.css 143 B
build/block-library/blocks/social-link/editor-rtl.css 309 B
build/block-library/blocks/social-link/editor.css 309 B
build/block-library/blocks/social-links/editor-rtl.css 727 B
build/block-library/blocks/social-links/editor.css 724 B
build/block-library/blocks/social-links/style-rtl.css 1.51 kB
build/block-library/blocks/social-links/style.css 1.51 kB
build/block-library/blocks/spacer/editor-rtl.css 346 B
build/block-library/blocks/spacer/editor.css 346 B
build/block-library/blocks/spacer/style-rtl.css 48 B
build/block-library/blocks/spacer/style.css 48 B
build/block-library/blocks/table-of-contents/style-rtl.css 83 B
build/block-library/blocks/table-of-contents/style.css 83 B
build/block-library/blocks/table/editor-rtl.css 394 B
build/block-library/blocks/table/editor.css 394 B
build/block-library/blocks/table/style-rtl.css 640 B
build/block-library/blocks/table/style.css 639 B
build/block-library/blocks/table/theme-rtl.css 152 B
build/block-library/blocks/table/theme.css 152 B
build/block-library/blocks/tag-cloud/editor-rtl.css 144 B
build/block-library/blocks/tag-cloud/editor.css 144 B
build/block-library/blocks/tag-cloud/style-rtl.css 266 B
build/block-library/blocks/tag-cloud/style.css 265 B
build/block-library/blocks/template-part/editor-rtl.css 368 B
build/block-library/blocks/template-part/editor.css 368 B
build/block-library/blocks/template-part/theme-rtl.css 113 B
build/block-library/blocks/template-part/theme.css 113 B
build/block-library/blocks/term-description/style-rtl.css 126 B
build/block-library/blocks/term-description/style.css 126 B
build/block-library/blocks/text-columns/editor-rtl.css 95 B
build/block-library/blocks/text-columns/editor.css 95 B
build/block-library/blocks/text-columns/style-rtl.css 165 B
build/block-library/blocks/text-columns/style.css 165 B
build/block-library/blocks/verse/style-rtl.css 98 B
build/block-library/blocks/verse/style.css 98 B
build/block-library/blocks/video/editor-rtl.css 441 B
build/block-library/blocks/video/editor.css 442 B
build/block-library/blocks/video/style-rtl.css 192 B
build/block-library/blocks/video/style.css 192 B
build/block-library/blocks/video/theme-rtl.css 134 B
build/block-library/blocks/video/theme.css 134 B
build/block-library/classic-rtl.css 179 B
build/block-library/classic.css 179 B
build/block-library/common-rtl.css 1.08 kB
build/block-library/common.css 1.08 kB
build/block-library/editor-elements-rtl.css 75 B
build/block-library/editor-elements.css 75 B
build/block-library/editor-rtl.css 11.8 kB
build/block-library/editor.css 11.8 kB
build/block-library/elements-rtl.css 54 B
build/block-library/elements.css 54 B
build/block-library/index.min.js 223 kB
build/block-library/reset-rtl.css 472 B
build/block-library/reset.css 472 B
build/block-library/style-rtl.css 15 kB
build/block-library/style.css 15 kB
build/block-library/theme-rtl.css 708 B
build/block-library/theme.css 712 B
build/block-serialization-default-parser/index.min.js 1.12 kB
build/block-serialization-spec-parser/index.min.js 2.87 kB
build/blocks/index.min.js 53 kB
build/commands/index.min.js 16.2 kB
build/commands/style-rtl.css 955 B
build/commands/style.css 952 B
build/components/index.min.js 229 kB
build/components/style-rtl.css 12.4 kB
build/components/style.css 12.4 kB
build/compose/index.min.js 12.7 kB
build/core-commands/index.min.js 3.09 kB
build/core-data/index.min.js 74.3 kB
build/customize-widgets/index.min.js 11 kB
build/customize-widgets/style-rtl.css 1.35 kB
build/customize-widgets/style.css 1.35 kB
build/data-controls/index.min.js 641 B
build/data/index.min.js 8.69 kB
build/date/index.min.js 18 kB
build/deprecated/index.min.js 458 B
build/dom-ready/index.min.js 325 B
build/dom/index.min.js 4.66 kB
build/edit-post/classic-rtl.css 578 B
build/edit-post/classic.css 580 B
build/edit-post/index.min.js 13.4 kB
build/edit-post/style-rtl.css 2.75 kB
build/edit-post/style.css 2.75 kB
build/edit-site/posts-rtl.css 7.46 kB
build/edit-site/posts.css 7.46 kB
build/edit-site/style-rtl.css 13.6 kB
build/edit-site/style.css 13.6 kB
build/edit-widgets/index.min.js 17.6 kB
build/edit-widgets/style-rtl.css 4.07 kB
build/edit-widgets/style.css 4.08 kB
build/editor/style-rtl.css 9.32 kB
build/editor/style.css 9.32 kB
build/element/index.min.js 4.82 kB
build/escape-html/index.min.js 537 B
build/format-library/index.min.js 8.05 kB
build/format-library/style-rtl.css 476 B
build/format-library/style.css 476 B
build/hooks/index.min.js 1.65 kB
build/html-entities/index.min.js 445 B
build/i18n/index.min.js 3.58 kB
build/is-shallow-equal/index.min.js 526 B
build/keyboard-shortcuts/index.min.js 1.31 kB
build/keycodes/index.min.js 1.46 kB
build/list-reusable-blocks/index.min.js 2.13 kB
build/list-reusable-blocks/style-rtl.css 852 B
build/list-reusable-blocks/style.css 852 B
build/media-utils/index.min.js 3.58 kB
build/notices/index.min.js 946 B
build/nux/index.min.js 1.62 kB
build/nux/style-rtl.css 749 B
build/nux/style.css 745 B
build/patterns/index.min.js 7.37 kB
build/patterns/style-rtl.css 687 B
build/patterns/style.css 685 B
build/plugins/index.min.js 1.86 kB
build/preferences-persistence/index.min.js 2.06 kB
build/preferences/index.min.js 2.9 kB
build/preferences/style-rtl.css 554 B
build/preferences/style.css 554 B
build/primitives/index.min.js 829 B
build/priority-queue/index.min.js 1.54 kB
build/private-apis/index.min.js 972 B
build/react-i18n/index.min.js 630 B
build/react-refresh-entry/index.min.js 9.47 kB
build/react-refresh-runtime/index.min.js 6.76 kB
build/redux-routine/index.min.js 2.7 kB
build/reusable-blocks/index.min.js 2.55 kB
build/reusable-blocks/style-rtl.css 256 B
build/reusable-blocks/style.css 256 B
build/rich-text/index.min.js 10.3 kB
build/router/index.min.js 5.42 kB
build/server-side-render/index.min.js 1.94 kB
build/shortcode/index.min.js 1.4 kB
build/style-engine/index.min.js 2.04 kB
build/token-list/index.min.js 581 B
build/url/index.min.js 3.9 kB
build/vendors/react-dom.min.js 41.7 kB
build/vendors/react-jsx-runtime.min.js 556 B
build/vendors/react.min.js 4.02 kB
build/viewport/index.min.js 965 B
build/vips/index.min.js 36.2 kB
build/warning/index.min.js 250 B
build/widgets/index.min.js 7.16 kB
build/widgets/style-rtl.css 1.16 kB
build/widgets/style.css 1.16 kB
build/wordcount/index.min.js 1.03 kB

compressed-size-action

Copy link
Contributor

@gigitux gigitux left a comment

Choose a reason for hiding this comment

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

Thanks for working on this! Overall the approach LGTM! I left a few comments

data: Item;
export type DataFormControlProps<
Item,
SupportsBulkEditing extends boolean = true,
Copy link
Contributor

Choose a reason for hiding this comment

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

It is just a personal preference, but I would prefer type composition:

export type DataFormControlProps< Item > = {
	data: Item;
	field: NormalizedField< Item >;
	onChange: ( value: Record< string, any > ) => void;
	hideLabelFromVision?: boolean;
	value: any;
};

export type WithBulkEditing< Item > = {
	data: Item | Item[];
};

And use in this way:

export const ParentEdit = ( {
	data,
	field,
	onChange,
}: DataFormControlProps< BasePost > & WithBulkEditing< BasePost > ) => {

We could do export already the composed type:

export type DataFormControlWithBulkEditingProps< Item > = DataFormControlProps< Item > & WithBulkEditing< Item >;

export const ParentEdit = ( {
	data,
	field,
	onChange,
}: DataFormControlWithBulkEditingProps< BasePost > 

const intersects = remainingRecords.every( ( item ) => {
return fieldDefinition.getValue( { item } ) === firstValue;
} );
return intersects ? firstValue : MIXED_VALUE;
Copy link
Contributor

@gigitux gigitux Dec 5, 2024

Choose a reason for hiding this comment

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

In regular layout we are passing the Symbol as value prop. I'm not sure that this is handled in the right way in the existent fields. For instance the feature image field:

const media = useSelect(
( select ) => {
const { getEntityRecord } = select( coreStore );
return getEntityRecord( 'root', 'media', value );
},
[ value ]
);

field,
onChange,
value,
}: DataFormControlProps< BasePost > ) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Featured Image doesn't support the bulk editing, should we set DataFormControlProps< BasePost, false >?

@@ -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 ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure about this logic.

I think that we need to decide whether bulk editing support should be opt-in or opt-out, regardless of whether the Edit function is present.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that we need to decide whether bulk editing support should be opt-in or opt-out, regardless of whether the Edit function is present.

Yeah I agree! I would lean towards optOut

@louwie17 louwie17 force-pushed the add/65685_bulk_editing_support_second_approach branch from 0228bbc to 0f3c00c Compare December 10, 2024 10:24
field,
onChange,
hideLabelFromVision,
}: DataFormControlProps< Item > ) {
value,
}: DataFormControlProps< Item > & WithBulkEditing< 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 don't understand this change. maybe I'm missing something when reading the diff though. Is "data" optional? where does "value" come from? I mean where are these type changes defined?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated the types.ts with the above: https://github.com/WordPress/gutenberg/pull/67584/files#diff-05cd2939bbf237a5dbcd116a0da9c9019b05ec1ceaa9d429c5fc872f149f63c4R192-R197

data is still passed in, but is either Item or Item | Item[] depending if combined with WithBulkEditing< Item >

Copy link
Contributor

Choose a reason for hiding this comment

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

Do we still need data?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah we still need data as there are still fields that rely on other data. Like the parent field which makes use of the post id and post type ( from the data object ) to filter the parent list. Or the slug field that uses permalink_template and link properties as well.

Currently I don't display these fields in bulk edit mode, as the logic may change a bit if there are multiple records passed down.
Unless there is a better way to pass these additional values down.

@@ -34,7 +33,7 @@ export default function DateTime< Item >( {
<VisuallyHidden as="legend">{ label }</VisuallyHidden>
) }
<TimePicker
currentTime={ value }
currentTime={ typeof value === 'symbol' ? '' : value }
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see the check about symbol in other controls like integer, why?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will update it, the TimePicker component it-self seemed to handle it gracefully, but its not expected.

return fieldDefinition.getValue( {
item: data,
} );
}, [ data, fieldDefinition ] );
Copy link
Contributor

Choose a reason for hiding this comment

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

If I'm not wrong this merging is probably duplicated in "regular" too. Should we extract it to a utility?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct, I can move it to a utility 👍

@@ -138,7 +157,13 @@ function PanelDropdown< Item >( {
) }
onClick={ onToggle }
>
<fieldDefinition.render item={ data } />
{ showMixedValue ? (
__( 'Mixed' )
Copy link
Contributor

Choose a reason for hiding this comment

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

In one of the issues, @jasmussen mentions the possibility to render a different "mixed" label/component based on the "type" of the control. So potentially we could defer this to the control/field itself at some point.

Copy link
Contributor

Choose a reason for hiding this comment

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

The context is loosely discussed here.

@@ -76,6 +81,7 @@ export function normalizeFields< Item >(
sort,
isValid,
Edit,
supportsBulkEditing,
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we mark supportsBulkEditing as required in the "NormalizedField" type?

@oandregal
Copy link
Member

Sharing some early thoughts after having given this a quick test (haven't looked at the code yet):

  • UI-wise, it'd be good to understand if all fields should be visible (even if disabled) in bulk editing vs only display those that can be edited as in this PR. cc @jameskoster
  • What makes a field not be editable in bulk? Specifically for this case: why we don't support bulk editing featured image, parent, or template? It seems to me that we shouldn't allow editing fields that are unique, like slug is — and they should be editable otherwise. This ties to validation as well. Or are there other cases in which we don't want to allow fields editable in bulk?
  • status doesn't work:
Screen.Recording.2024-12-10.at.11.57.54.mov

@@ -64,6 +64,11 @@ export function normalizeFields< Item >(
);
};

const supportsBulkEditing =
Copy link
Member

@oandregal oandregal Dec 10, 2024

Choose a reason for hiding this comment

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

Why bulk editing is tied to the field type or even having a custom edit function? In my view, this is business/consumer logic: users can't edit fields that are unique. That is unrelated to any field type or what kind of edit function is in place.

I was looking for something like this in the fields package instead (in the slug field, for example). Additionally, if we agree that the only case in which a field cannot be bulk edited is when it needs to be unique, I wonder how can we tie this to validation. For example, when editing the field, the user shouldn't be able to add a value that is in use by another item.

Copy link
Member

Choose a reason for hiding this comment

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

The way I think about this is the following: any field is bulk editable regardless of their fieldType/edit function, unless it provides a specific opt-out (it's an unique field).

Copy link
Contributor

Choose a reason for hiding this comment

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

Some fields could be unique for instance or things like that.

@louwie17
Copy link
Contributor Author

Sharing some early thoughts after having given this a quick test (haven't looked at the code yet):

Thanks :) I will be making some updates after Riad's review.

  • What makes a field not be editable in bulk? Specifically for this case: why we don't support bulk editing featured image, parent, or template? It seems to me that we shouldn't allow editing fields that are unique, like slug is — and they should be editable otherwise. This ties to validation as well. Or are there other cases in which we don't want to allow fields editable in bulk?

Yeah good question, I think limiting to unique fields only makes sense and defaulting to fields supporting bulk editing. In terms of additional use cases, it may also be a question for @jameskoster. For example there is nothing limiting us from displaying the featured image field compared to the rest, but is this something users will actually use within bulk edit mode?

This also brings up the question of whether we want to disable bulk editing purely within the layout ( although the field may support it ). Something @gigitux suggested in my last version to move the supportsBulkEditing to the SimpleFormField type:

export type SimpleFormField = {
id: string;
layout?: 'regular' | 'panel';
labelPosition?: 'side' | 'top' | 'none';
};

  • status doesn't work:

Looks like I forgot to update this line with editedRecord as well: https://github.com/WordPress/gutenberg/pull/67584/files#diff-4ed486e973856f952d2b80aea381fc1f5a1d0004995a0b68554147b02cee0b55R106

@jameskoster
Copy link
Contributor

UI-wise, it'd be good to understand if all fields should be visible (even if disabled) in bulk editing vs only display those that can be edited as in this PR. cc @jameskoster

I see this panel as an Inspector/Edit interface, so it seems reasonably clear to me that we should show uneditable fields while inspecting a single record. I don't have a strong feeling about bulk editing though, it may be something we decide field by field. For instance 'file size' in the media library should show a range based on the selection, which might be useful, whereas 'File type: Mixed' is less useful.

That said, it seems okay to hide uneditable fields during bulk selection for now :)

Yeah good question, I think limiting to unique fields only makes sense [...] For example there is nothing limiting us from displaying the featured image field compared to the rest, but is this something users will actually use within bulk edit mode?

Prohibiting bulk edits to unique fields seems like a good place to start.

I agree it's unlikely a user would want to use the same featured image, title, etc. on multiple records, but I also acknowledge it's not impossible, so I don't mind allowing that initially.

@louwie17
Copy link
Contributor Author

Just pushed up a couple more changes, but not entirely ready for another round of reviews.
Will follow up tomorrow.

Copy link

github-actions bot commented Dec 10, 2024

Flaky tests detected in b9ca481.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/12300375146
📝 Reported issues:

@oandregal
Copy link
Member

Prohibiting bulk edits to unique fields seems like a good place to start.

I agree it's unlikely a user would want to use the same featured image, title, etc. on multiple records, but I also acknowledge it's not impossible, so I don't mind allowing that initially.

I agree with this direction.

How do we distill that into API? As I mentioned above, what I'd expect to see here is something along the lines of an unique prop in the Fields API:

[
  {
    id: 'slug',
    unique: true
  }
]

This new prop will control the bulk editing experience, and will also be used in validation as well (to be explored in a follow-up PR).

We could introduce something specific for things that are not unique but want bulk editing disabled anyway, like a bulkEditing prop in the Fields API. All together, it could look like this (none of these fields would be bulk editable):

[
  {
    id: 'slug',
    unique: true
  },
  {
    id: 'feature_image',
    bulkEditing: false
  }
]

I think I'd start by implementing the unique prop: that's something we want and ties to other work we're already doing/planning to start around validation. Starting by bulkEditing would be ok, but I fear we'll be missing the bigger picture and the connection with validation.

@gigitux
Copy link
Contributor

gigitux commented Dec 11, 2024

I think I'd start by implementing the unique prop: that's something we want and ties to other work we're already doing/planning to start around validation. Starting by bulkEditing would be ok, but I fear we'll be missing the bigger picture and the connection with validation.

This is a great way to connect the dots! Thanks for proposing this! I'm entirely agree with this approach!

@louwie17 louwie17 force-pushed the add/65685_bulk_editing_support_second_approach branch from f6e3ec4 to 6c629e6 Compare December 12, 2024 16:01
id
)
)
: null,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will replace this with getEditedEntityRecords once #67872 is merged.

@louwie17
Copy link
Contributor Author

I think I'd start by implementing the unique prop: that's something we want and ties to other work we're already doing/planning to start around validation. Starting by bulkEditing would be ok, but I fear we'll be missing the bigger picture and the connection with validation.

Yeah I am totally onboard with using the unique prop, as this does depict our current use case better.
I updated it as part of this commit: 6c629e6, although I did have one follow up question @oandregal: When you are referring to the Fields API, do you mean the place where I have added now, as part of the Field type: https://github.com/WordPress/gutenberg/pull/67584/files#diff-05cd2939bbf237a5dbcd116a0da9c9019b05ec1ceaa9d429c5fc872f149f63c4R164-R168
Or adding it to the SimpleFormField type, which also defines the layout and label position?

export type SimpleFormField = {
id: string;
layout?: 'regular' | 'panel';
labelPosition?: 'side' | 'top' | 'none';
};

Both places is also a valid answer :)

@oandregal
Copy link
Member

When you are referring to the Fields API, do you mean the place where I have added now, as part of the Field type

Yes, what you did.

@louwie17 louwie17 marked this pull request as ready for review December 13, 2024 12:37
Copy link

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 props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: louwie17 <[email protected]>
Co-authored-by: gigitux <[email protected]>
Co-authored-by: youknowriad <[email protected]>
Co-authored-by: oandregal <[email protected]>
Co-authored-by: jasmussen <[email protected]>
Co-authored-by: jameskoster <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@louwie17
Copy link
Contributor Author

@gigitux, @youknowriad, and @oandregal I moved this PR out of draft now and should be ready for a re-review. There is just one thing that is dependant on the hook introduced in this PR: #67872, that I will have to update once that is merged.

@oandregal
Copy link
Member

Took a look at this one. Haven't been able to review all use cases (combined fields).

Function Input Comment
Edit data (single item or array) + consolidated value (can be mixed)
render data (single item) If there's multiple items, we display MIXED and do not call render.
isVisible data (single item or array) Need to fix type to support arrays. Need to update all consumers that use it to suport arrays (example) or call isVisible for every item.
isValid data (single item) This is called by the consumer via isItemValid utility (code). What do we do with this one?

I wonder how can we improve the experience for consumers in the Edit function. Do we really need to pass both the data (single + array) and the consolidated value? We may run into the following situations:

  1. DataForm with a single item. Edit receives a single item.
  2. DataForm with multiple items and, for a given field, all are equal. Edit receives a single item.
  3. DataForm with multiple items and, for a given field, there are mixed results. This is the crux we need to solve.

Thinking out loud about 3. We don't really know anything about the data Edit expects and how it'll operate with it. That's why mixed is difficult: we can't simple pass empty or null because those may have a special meaning for Edit. And that's the only case in which Edit would need access to the array of data.

What if we just pass the array in the 3 scenario and we never pass MIXED to consumers? If Edit receives an array, it needs to resolve the values itself. We'd also have to provide implementations for the controls we bundle (integer, datetime, etc.): we could set them to "empty value" in that scenario, for example.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] DataViews Work surrounding upgrading and evolving views in the site editor and beyond [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants