Dataform bulk editing support #67344
Open
+139
−56
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
louwie17
commented
Nov 27, 2024
| ( fieldDefinition ) => fieldDefinition.id === fieldId | ||
| )?.supportsBulk; | ||
| } ); | ||
| } |
There was a problem hiding this comment.
Checks if 'any' of the nested fields support bulk, returns 'true' in that case.
louwie17
commented
Nov 27, 2024
| @@ -111,6 +113,10 @@ function PanelDropdown< Item >( { | |||
| [ popoverAnchor ] | |||
| ); | |||
|
|
|||
| const fieldValue = fieldDefinition.getValue( { item: data } ); | |||
| const showMixedValue = | |||
| isBulkEditing && ( fieldValue === undefined || fieldValue === '' ); | |||
There was a problem hiding this comment.
This is where having a central intersected object falls short, and why I am leaning towards moving the array of items down so the field layout has more control and also optionally the field.
As in this case the getValue for title, returns an empty string if it isn't defined. While empty string may be a valid case for another field and different from undefined or null.
There was a problem hiding this comment.
yeah value undefined or '' might be valid values, this is why I suggested the "Symbol"
There was a problem hiding this comment.
@youknowriad Where was it that you suggested Symbol?
And are you thinking we did use it to define the Mixed value. That in the getIntersectingValues function for example when it is a mixed value we set it to Symbol.for( 'unique dataform mixed value key' )? and than check for that in the above? or are you thinking something else?
I did try the above, but ran into this error Cannot use 'in' operator to search for 'rendered' in Symbol(DATAFORM_MIXED_VALUE) within the getItemTitle function. So in order to make this work I did probably have to do what I suggested above:
I am leaning towards pushing the intersecting logic down to the field view and rely on getValue, but this will cascade into quite a few other changes.
louwie17
commented
Nov 27, 2024
| @@ -15,6 +15,7 @@ const authorField: Field< BasePostWithEmbeddedAuthor > = { | |||
| id: 'author', | |||
| type: 'integer', | |||
| elements: [], | |||
| supportsBulk: true, | |||
There was a problem hiding this comment.
Should a type also be able to decide if it supports bulk or not?
Maybe only if no custom Edit is being passed in?
|
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 If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Size Change: +420 B (+0.02%) Total Size: 1.83 MB
ℹ️ View Unchanged
|
|
@gigitux, @youknowriad, @oandregal this PR is ready for review, it is an initial stab at the bulk editing. I left some open questions and it will likely require some follow up, depending on how integral we want to make it. |
|
Flaky tests detected in ac03ed1. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/12122182106
|
|
Thanks for working on this! I'm considering whether For example, the slug field doesn't support bulk editing. An extension might need to have the same slug for different entities, given that the |
youknowriad
reviewed
Nov 29, 2024
packages/dataviews/src/types.ts
Outdated
| /** | ||
| * Whether the action can be used as a bulk action. | ||
| */ | ||
| supportsBulk?: boolean; |
There was a problem hiding this comment.
Should we be more explicit? supportsBulkEditing or something like that. We may have bulk actions or something else.
There was a problem hiding this comment.
I like the supportsBulkEditing, done in 4df0165
| @@ -111,6 +113,10 @@ function PanelDropdown< Item >( { | |||
| [ popoverAnchor ] | |||
| ); | |||
|
|
|||
| const fieldValue = fieldDefinition.getValue( { item: data } ); | |||
| const showMixedValue = | |||
| isBulkEditing && ( fieldValue === undefined || fieldValue === '' ); | |||
There was a problem hiding this comment.
yeah value undefined or '' might be valid values, this is why I suggested the "Symbol"
| } ); | ||
| if ( intersects ) { | ||
| intersectingValues[ key ] = firstRecord[ key ]; | ||
| } |
There was a problem hiding this comment.
Should we be more explicit? supportsBulkEditing or something like that. We may have bulk actions or something else.
There was a problem hiding this comment.
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?
youknowriad
added
[Type] Enhancement
louwie17
force-pushed
the
add/65685_bulk_editing_support
branch
from
7fb76cf
to
ac03ed1
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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?
DataForm allows an array of items to be passed in.
When an array of items is passed in it does a couple things:
bulkEditingand only shows fields withbulkEditingsupport.Some questions/follow up:
getValue, but this will cascade into quite a few other changes.getValueand pass thedataobject by default, this will break if thedataobject can also be an array.onChangemethod with the intersecting changes only make sense? or are we better off passing the array back with the updates.Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast
bulk-editing.mp4