Implement bulk selection in editor #369

brookewp · 2025-02-13T20:55:12Z

Updated visual:

Adds optional supports_bulk to input variable which will add an option to bulk select data in the editor:

Screen.Recording.2025-02-14.at.3.52.52.PM.mov

To support this, the endpoint must allow for multiple items to be queried. I've updated the Airtable Integration and the Art Institute example to accept single or multiple IDs.

Instead of returning an object with a single value, it'll return the object with comma separated strings. For the examples I've mentioned above, this includes using a comma separated string entered into the manual input (Thanks @alecgeatches for pointing this out)

The solution I've come up with is adding a total count, and hiding the DataViews button, and adding our own within the Modal instead, so we can control when it's shown, and make sure it doesn't affect blocks that don't support bulk select.

Signed-off-by: brookewp <[email protected]>

alecgeatches · 2025-02-18T18:39:19Z

Howdy @brookewp! Just started testing, and I really like the UI for selection! I noticed a couple of things so far:

Items can only be multi-selected across the same page:

When changing pages, the selection resets. I'm not sure if this is a data views limitation, but it seems likely users will need to select from items that appear across multiple pages.
Manual input on a multiselect still only asks for a single ID.

Maybe we add below the "ART ID" label to include "Comma-separate multiple values" and handle that? Just seems useful to cover this case, but not super important.

alecgeatches · 2025-02-18T18:41:49Z

example/rest-api/art-institute/art-institute.php

+			if ( isset( $input_variables['id'] ) ) {
+				$ids[] = $input_variables['id'];
+			} else {
+				foreach ( $input_variables as $input ) {


This foreach seems a bit complicated, what case does this cover?

Ya 😬 This is to ensure that a single item can still be selected and transforms it into an array to handle either a single item or multi-item selection.

In the code, it first checks if an 'id' key and if it's there, it creates a single-item array. Otherwise, it filters the array of input variables for items with 'id' and maps them to extract the id values.

I added comments and tried to make the code easier to read, but not sure if I've achieved it. If you have any suggetions, please share!

remote-data-blocks/example/rest-api/art-institute/art-institute.php

Lines 35 to 43 in 8ca8312

// Extract artwork IDs from input variables:

// - If a single input is provided directly, wrap the id in an array

// - Otherwise, process an array of inputs to extract the id fields

$ids = isset($input_variables['id'])

? [$input_variables['id']]

: array_map(

fn($input) => is_array($input['id']) ? reset($input['id']) : $input['id'],

array_filter($input_variables, fn($input) => is_array($input) && isset($input['id']))

);

alecgeatches · 2025-02-18T18:44:43Z

example/rest-api/art-institute/art-institute.php

@@ -40,20 +65,28 @@ function register_aic_block(): void {
 			],
 		],
 		'output_schema' => [
-			'is_collection' => false,
-			'path' => '$.data',
+			'is_collection' => true,


Does supports_bulk = true imply is_collection = true?

I actually should rephrase, I think that's the wrong terminology. Does a block having supports_bulk = true on the selection query imply that the get query needs to be is_collection = true?

Does a block having supports_bulk = true on the selection query imply that the get query needs to be is_collection = true?

The answer is yes, but I'm not sure if it should be. So great question! 😄

To use our existing setup, it needs to be true so we can handle multiple returned results, otherwise, we only expect a single result to be returned.

But now I'm curious why we haveis_collection in the first place. In trunk without multi-select, if I set the collection to true for this example, we can still display the single result without issue.

To me, it seems safer to handle everything as an array so we can either map through one or more results and not need the is_collection.

But now I'm curious why we have is_collection in the first place.

Short answer: To provide a consistent response shape from the remote-data REST endpoint.

Longer answer involves the fact that JSONPath always returns an array, even for single values:

We use is_collection when parsing response data to understand if the return value is a single item or, coincidentally a collection with only one member.

Thanks for answering this! If the response always returns an array, even for single values, then couldn't we parse everything as an array and not have to rely on the config to know if it's a single item in an array or multiple items?

I think that might be possible if we merge the concept of single and loop blocks (which we probably should!). Right now, it just helps to prevent lots of inspection of the response to determine if its a collection or not.

alecgeatches · 2025-02-18T19:00:09Z

inc/Editor/DataBinding/BlockBindings.php

+				return self::loop_block_render_callback( $block_attributes, '', $block );
+			}
+			// For non-WP_Block instances, fallback to single result
+			$index = 0;


@brookewp Can you help me understand why the $block instanceof WP_Block check and this code exists? When does the type of block coming in make a difference here?

I see loop_block_render_callback() requires a WP_Block, but I'm not sure why a block comes in as a WP_Block versus a array, and what that means.

At the start of the function, I saw this helpful comment from Hew:

remote-data-blocks/inc/Editor/DataBinding/BlockBindings.php

Lines 168 to 177 in 8ca8312

public static function get_value( array $source_args, WP_Block|array $block ): ?string {

// We may be passed a block instance (by core block bindings) or a block

// array (by our hooks into the Block Data API).

if ( $block instanceof WP_Block ) {

$block_context = $block->context[ self::$context_name ] ?? [];

$block_attributes = $block->attributes;

} else {

$block_context = $block['context'][ self::$context_name ] ?? [];

$block_attributes = $block['attributes'] ?? [];

}

From my understanding, when we are using standard WP methods for rendering blocks, we end up with a WP_Block instance. But if we are doing something custom (e.g., our hooks) before it's rendered in the editor or on the frontend, we end up with an array for more flexibility. But either way, it'll eventually become a WP_Block through the core process. So, I don't think it needs strict typing, I was just adhering to what was established initially in this function.

That's correct, it's mainly to accommodate the Block Data API. It also happens to make testing easier.

alecgeatches · 2025-02-18T19:04:46Z

inc/Integrations/Airtable/AirtableIntegration.php

@@ -93,14 +94,47 @@ public static function get_query( AirtableDataSource $data_source, array $table
 		];

 		$output_schema = [
-			'is_collection' => false,
+			'is_collection' => true,


The transformation here and above makes me wonder if we should write all getter queries to assume is_collection to be true, and require them to implement multi-select gets.

A couple of ideas:

It's possible that some API only can do GET requests one at a time. Maybe we could implement an example of an API that doesn't support multi-select, but makes multiple requests internally in the get_query call.

OR we instead have a flag that indicates a request only supports single queries supports_multi = false, so all queries are multi-select by default.

I think most APIs will support bulk selection, and it would be great if we could have this all supported and working out of the box without needing to add these extra flags.

It's possible that some API only can do GET requests one at a time. Maybe we could implement an example of an API that doesn't support multi-select, but makes multiple requests internally in the get_query call.

You may have seen this already in my earlier comment regarding is_collection, but I believe the boolean determines what is returned, so either a collection of results or a single result:

remote-data-blocks/docs/extending/query.md

Line 147 in b842c6f

- `is_collection` (optional, default `false`): A boolean indicating whether the response data is a collection. If false, only a single item will be returned.

Expanding on my comment, I wonder why, instead of relying on the config/asking the user to tell us if it's a collection, we aren't checking for an array like this:

diff --git a/inc/Config/QueryRunner/QueryRunner.php b/inc/Config/QueryRunner/QueryRunner.php index 436ef480..4f12998c 100644 --- a/inc/Config/QueryRunner/QueryRunner.php +++ b/inc/Config/QueryRunner/QueryRunner.php @@ -223,7 +223,7 @@ class QueryRunner implements QueryRunnerInterface { // is_collection and unwrap if necessary. $parser = new QueryResponseParser(); $results = $parser->parse( $response_data, $output_schema ); - $results = $is_collection ? $results : [ $results ]; + $results = is_array( $results ) ? $results : [ $results ]; $metadata = $this->get_response_metadata( $query, $raw_response_data['metadata'], $results );

Keeping the results to an array for consistency, but not limiting to the first item.

As a side note to potential single API requests, I don't think we would want to connect multiple selections to multiple GET requests, as I think we'd run into rate limiting issues pretty quickly

alecgeatches · 2025-02-18T19:11:26Z

inc/Integrations/Airtable/AirtableIntegration.php

+					// Single record case
+					$record_ids[] = $input_variables['record_id'];
+				} else {
+					// Multiple records or array case


Maybe refactor this with the other use in the art institute block into a reusable function? I'm still a bit confused on what's happening here, but it seems like an operation we need multiple places.

alecgeatches · 2025-02-18T19:12:48Z

inc/REST/RemoteDataController.php

@@ -61,10 +61,6 @@ public static function execute_query( WP_REST_Request $request ): array|WP_Error
 		$block_config = ConfigStore::get_block_configuration( $block_name );
 		$query = $block_config['queries'][ $query_key ];

-		// The frontend might send more input variables than the query needs or
-		// expects, so only include those defined by the query.
-		$query_input = array_intersect_key( $query_input, $query->get_input_schema() );


Curious why this no longer is necessary

Good catch! I initially removed it because it wasn't working with multi-selection, but I intended to circle back to it. I added it back here, with a check for an array to include multi-selection: a0ff2c9 (#369)

I'm not 100% sure what the benefit of it is, aside from making the request smaller. 🤔

I'm not 100% sure what the benefit of it is, aside from making the request smaller.

Some APIs throw an error when they receive unexpected input. (Our own APIs do this.)

alecgeatches

I left a comment above with a couple of potential UI changes, and then a bunch of questions below. Overall this is really cool, and I'm curious to see if we can change the configuration to get this "for free" for most queries, rather than having it be an add-on.

Signed-off-by: brookewp <[email protected]>

brookewp · 2025-02-19T03:54:24Z

Thanks for all of the thoughtful comments @alecgeatches!

Items can only be multi-selected across the same page:
When changing pages, the selection resets. I'm not sure if this is a data views limitation, but it seems likely users will need to select from items that appear across multiple pages.

Looks like a bug in DataViews 😞

Manual input on a multiselect still only asks for a single ID.
Maybe we add below the "ART ID" label to include "Comma-separate multiple values" and handle that? Just seems useful to cover this case, but not super important.

To be honest, I didn't even think we'd include multi-select here 🤯 but it's a good point!

chriszarate · 2025-02-19T15:11:09Z

example/rest-api/art-institute/art-institute.php

@@ -129,6 +157,7 @@ function register_aic_block(): void {
 			[
 				'query' => $search_art_query,
 				'type' => 'search',
+				'supports_bulk' => true,


I think it's extremely important for the queries themselves to declare their support for various UI features—not the config that surrounds the queries. We should give the input variable that accepts multiple IDs a special type, just as we have for search terms and pagination input variables.

Reasons:

It reduces the complexity of the block config, which is already complex.

We keep the configuration very close to the thing it is describing (the input variable), instead of far away.

We reduce the likelihood of having to repeat this configuration value in multiple places.

It matches the pattern we are already using.

After reading a bit more code, I think I'm unclear which approach you want to take:

a. Provide an array of n IDs to a query and have it dispatch a single request for all of them. The query must support multiple IDs as input.
b. Given an array of n IDs, execute the query n times and use a "loop" block to render them.
c. Dynamically choose approach A or B, depending on whether the query supports multiple IDs as input.

Can you elaborate? If it's B or C, doesn't every query "support" bulk selection? Why would it even be an option?

My approach was more toward option A, which allows you to enable bulk selection for the query (which would need to be updated to align with your earlier comment) if you have an API that accepts multiple inputs. Since you can submit a single input to these endpoints, I didn't think we'd need to have a specific input variable just for multiple selections.

I also thought executing the query multiple times wouldn't be ideal, as someone may be blocked for making too many subsequent requests.

But I'd like to hear your thoughts on which direction we should aim for. 🙂

@chriszarate I moved the flag to the input variable for the render/display queries in 4df9b41 (#369). Side notes:

I didn't make it a special type, because I thought we might still want to know the type, 'id' in this case (which I use in ItemList)

We still need to communicate this capability to the selection queries, so I added a check in the config registry

I also limited bulk selection so only one input variable can be present, as I'm not certain how we want to factor

Signed-off-by: brookewp <[email protected]>

brookewp · 2025-02-21T01:46:35Z

@alecgeatches I've implemented multi-select via comma separated IDs for the manual input.

Also, it seems we can select across multiple pages by updating this to a controlled component. I haven't pushed those changes up yet, as I'm not sure how to deal with the incorrect number of selected items managed by DataViews. 🤔 The 'best' solution is probably adding the count into the header and hiding the existing one from DataViews. It just won't be the ideal placement.

Signed-off-by: brookewp <[email protected]>

brookewp · 2025-02-21T21:30:02Z

For the bulk selection pagination issue, the problems:

navigating to other pages clears the selection
when fixing this by using it as a controlled component, we still have issues where the buttons will disappear unless you go back to pages with selections or you make a new selection
the total count at the bottom is per page, but I don't think it's entirely clear

The solution I've come up with is adding a total count, and hiding the DataViews button, and adding our own within the Modal instead, so we can control when it's shown, and make sure it doesn't affect blocks that don't support bulk select.

Signed-off-by: brookewp <[email protected]>

brookewp · 2025-02-22T00:10:45Z

Maybe a 'Clear all' button could be helpful?

brookewp added 12 commits February 13, 2025 12:53

Add multi-select for airtable

5048842

Signed-off-by: brookewp <[email protected]>

Allow for single queries (field shortcodes)

0c64f94

Signed-off-by: brookewp <[email protected]>

Ensure existing items can be found for shortcodes

29d0d48

Signed-off-by: brookewp <[email protected]>

Update styling

94b0f0e

Signed-off-by: brookewp <[email protected]>

Update art instituate example

4a6a4bf

Signed-off-by: brookewp <[email protected]>

Add check for bulk selection

101e28d

Signed-off-by: brookewp <[email protected]>

Update type

74d95b5

Signed-off-by: brookewp <[email protected]>

Merge branch 'trunk' into add/multi-select

e941d40

Lint fixes and placeholder update

bf2e9f9

Signed-off-by: brookewp <[email protected]>

Update styles

30f76d6

Signed-off-by: brookewp <[email protected]>

Update types

3c24f4d

Signed-off-by: brookewp <[email protected]>

Show block title instead of query type

ca99ce5

Signed-off-by: brookewp <[email protected]>

brookewp marked this pull request as ready for review February 15, 2025 00:28

brookewp requested a review from alecgeatches February 18, 2025 17:04

alecgeatches reviewed Feb 18, 2025

View reviewed changes

alecgeatches requested changes Feb 18, 2025

View reviewed changes

brookewp added 3 commits February 18, 2025 12:44

Clean up id check for aic example

b37bab3

Signed-off-by: brookewp <[email protected]>

Try to make aic id check more concise

8ca8312

Signed-off-by: brookewp <[email protected]>

Add back input variable schema check

a0ff2c9

Signed-off-by: brookewp <[email protected]>

chriszarate reviewed Feb 19, 2025

View reviewed changes

brookewp added 3 commits February 20, 2025 10:49

Merge branch 'trunk' into add/multi-select

750aca8

Move flag to input variable

4df9b41

Signed-off-by: brookewp <[email protected]>

Return to former structure with a comma separated list instead of array

bf0fc5c

Signed-off-by: brookewp <[email protected]>

brookewp added 5 commits February 20, 2025 17:12

Add back loop check and add specific check for bulk support

21a21e5

Signed-off-by: brookewp <[email protected]>

Clean up and move check to function

f16f69f

Signed-off-by: brookewp <[email protected]>

Spacing fixes and type update

d68541c

Signed-off-by: brookewp <[email protected]>

Remove random space

9d6561e

Signed-off-by: brookewp <[email protected]>

Merge branch 'trunk' into add/multi-select

2e2863c

brookewp added 3 commits February 21, 2025 12:04

Implement fix to select on multiple pages

63b5aab

Signed-off-by: brookewp <[email protected]>

Move buttons to modal

f5a3aae

Signed-off-by: brookewp <[email protected]>

Fix footer stacking

1e0427d

Signed-off-by: brookewp <[email protected]>

Merge branch 'trunk' into add/multi-select

93629e7

brookewp requested review from chriszarate and alecgeatches February 21, 2025 21:34

brookewp added 3 commits February 21, 2025 14:34

Dataviews fixes

fc9d7a2

Signed-off-by: brookewp <[email protected]>

Add images to show in search modal

17682bf

Signed-off-by: brookewp <[email protected]>

Ensure content isn’t hidden under fixed footer

f8664bd

Signed-off-by: brookewp <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement bulk selection in editor #369

Implement bulk selection in editor #369

brookewp commented Feb 13, 2025 •

edited

Loading

alecgeatches commented Feb 18, 2025

alecgeatches Feb 18, 2025 •

edited

Loading

brookewp Feb 18, 2025

alecgeatches Feb 18, 2025

alecgeatches Feb 18, 2025

brookewp Feb 18, 2025

chriszarate Feb 19, 2025

brookewp Feb 19, 2025

chriszarate Feb 20, 2025

alecgeatches Feb 18, 2025

alecgeatches Feb 18, 2025

brookewp Feb 18, 2025 •

edited

Loading

chriszarate Feb 19, 2025

alecgeatches Feb 18, 2025

alecgeatches Feb 18, 2025

brookewp Feb 18, 2025 •

edited

Loading

brookewp Feb 18, 2025

alecgeatches Feb 18, 2025

alecgeatches Feb 18, 2025

brookewp Feb 19, 2025

chriszarate Feb 19, 2025

alecgeatches left a comment

brookewp commented Feb 19, 2025

chriszarate Feb 19, 2025

chriszarate Feb 19, 2025

brookewp Feb 19, 2025

brookewp Feb 21, 2025

brookewp commented Feb 21, 2025

brookewp commented Feb 21, 2025

brookewp commented Feb 22, 2025

	// Extract artwork IDs from input variables:
	// - If a single input is provided directly, wrap the id in an array
	// - Otherwise, process an array of inputs to extract the id fields
	$ids = isset($input_variables['id'])
	? [$input_variables['id']]
	: array_map(
	fn($input) => is_array($input['id']) ? reset($input['id']) : $input['id'],
	array_filter($input_variables, fn($input) => is_array($input) && isset($input['id']))
	);

	public static function get_value( array $source_args, WP_Block\|array $block ): ?string {
	// We may be passed a block instance (by core block bindings) or a block
	// array (by our hooks into the Block Data API).
	if ( $block instanceof WP_Block ) {
	$block_context = $block->context[ self::$context_name ] ?? [];
	$block_attributes = $block->attributes;
	} else {
	$block_context = $block['context'][ self::$context_name ] ?? [];
	$block_attributes = $block['attributes'] ?? [];
	}

Implement bulk selection in editor #369

Are you sure you want to change the base?

Implement bulk selection in editor #369

Conversation

brookewp commented Feb 13, 2025 • edited Loading

alecgeatches commented Feb 18, 2025

alecgeatches Feb 18, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brookewp Feb 18, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brookewp Feb 18, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alecgeatches left a comment

Choose a reason for hiding this comment

brookewp commented Feb 19, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brookewp commented Feb 21, 2025

brookewp commented Feb 21, 2025

brookewp commented Feb 22, 2025

brookewp commented Feb 13, 2025 •

edited

Loading

alecgeatches Feb 18, 2025 •

edited

Loading

brookewp Feb 18, 2025 •

edited

Loading

brookewp Feb 18, 2025 •

edited

Loading