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

RFC: Enable full customization of Query Loop block inspector controls #43675

Draft
wants to merge 4 commits into
base: trunk
Choose a base branch
from

Conversation

sunyatasattva
Copy link
Contributor

What?

This PR enables developers to completely extend the Query Loop block options, deciding which settings should be available and which should not.

Why?

Developers might want to create Query Loop block variations which are more specific to their environment, and, as such, don't require all of the settings available by default on the Query Loop block. Even further, some of these settings might break block variations.

How?

This PR proposes a way to enable this customization that is completely backwards compatible, and enables a simple API for developers to hook into and extend the settings.

  1. A new attribute called disabledInspectorControls has been added to the block. Developers can pass an array of strings corresponding to the keys of the settings that they want to disable to not render them on the inspector sidebar. Since we are not using TypeScript, perhaps adding a clear JSDoc documentation of the keys available by default could be helpful. However, no harm is done if wrong strings are passed, and the block behaves normally when the attribute is an empty array or undefined, preserving backwards compatibility.
  2. disabledInspectorControls also accept a list of strings corresponding to the collapsed filters. However, since filters have been moved to a ToolsPanel, in order to allow developers to extend that panel and completely integrate new optional filters to the inspector, a new WordPress filter has been added (provisional name: editor.QueryLoop.FilterControls, naming convention inspired by filters found in components/Autocomplete).
  3. Taking advantage of these changes, all the controls have been refactored out of the inspector-control/index.js file into their own file, cutting the size of that file by more than half and making it more readable. Now, it is much clearer what each component is responsible for, and what properties are required, instead of having all the logic mixed into one component.

Note
In the interest of keeping my edits as minimal as possible, I preserved the already existing default exports for editor components which had been refactored, and added named exports below them.

Basically, the default exports have no knowledge of the context in which they are rendered, which might be a good thing. However, considering they are unlikely to be used in different context, they could arguably contain their rendering logic, and we could simplify the code even further. But I have no strong opinions on this, if we want to leave them as they are and enable reusability in different contexts.


@gziolo @ntsekouras I'm publishing this as a Draft PR to get your comments before I submit it for full review, in case you disagree with some of the approach taken here.

Testing Instructions

Warning
Testing instructions are WIP

  1. Smoke test the default Query Loop block and verify all settings and filters still work.
  2. Create a Query Loop variation (or just instantiate the block programmatically) by adding the disabledInspectorControls attribute, like so:
registerBlockVariation( 'core/query', {
  name: 'test/my-query',
  title: 'My Query',
  attributes: {
    disabledInspectorControls: [ 'InheritFromTemplate', 'PostType' ],
  },
  innerBlocks: [
    [
      'core/post-template',
      {},
      [ [ 'core/post-title' ], [ 'core/post-featured-image' ] ],
    ],
    [ 'core/query-pagination' ],
    [ 'core/query-no-results' ],
  ],
  scope: [ 'block', 'inserter' ],
} );
  1. Add it to the Editor.
  2. Verify that the listed settings are not available in the Inspector.
  3. Add a WordPress filter to extend the Inspector Filters, like so:
const MY_FILTERS = {
  MyFilter: ( props ) => (
    <ToolsPanelItem
      hasValue={ /* ... */ }
      label={ 'My Filter' }
      onDeselect={ /* ... */ }
    >
      <TextControl
        label={ 'My Filter' }
        value={ /* ... */ }
        onChange={ /* ... */ }
      />
    </ToolsPanelItem>
  ),
};

const withMyFilters = ( filtersMap, props ) => {
  return {
    ...filtersMap,
    ...MY_FILTERS,
  }
};

addFilter(
  'editor.QueryLoop.FilterControls',
  'core/query',
  withMyFilters
);
  1. Verify your filter appears on the Filters ToolsPanel dropdown.

Screenshots or screencast

Screenshot 2022-08-29 at 07 40 39

Notice the missing settings on the right, and a new filter within the Filters ToolsPanel.

@ntsekouras
Copy link
Contributor

Thanks for the PR @sunyatasattva!

A couple of thoughts:

  1. We could refactor some filters to their own files(although many are already), but IMO it should be done in its own PR without mixing them with more important updates.
  2. Regarding the hideControls, have you checked my PR here: https://github.com/WordPress/gutenberg/pull/43632/files#diff-7b59aac98ed861c60efc823b7e874d03b9ad261f183c6ea7986cac4bdb5b0159R61? In general such a prop shouldn't be serialized in the block markup and as you can see in my PR there isn't the need for it
  3. About editor.QueryLoop.FilterControls I don't think we need a filter. In the PR that I introduced the ToolsPanel, I initially had added the panelId but removed it for a separate PR. I think it will be possible to just add the ToolsPanel to a panelId in the same way block supports panel work.

@gziolo
Copy link
Member

gziolo commented Aug 30, 2022

@sunyatasattva, nice progress on making the Query Loop block easy to customize for different post types 🤩

Let's try to break down this work into smaller task like the one proposed by @ntsekouras in #43632 so we address individual aspects separately.

Taking advantage of these changes, all the controls have been refactored out of the inspector-control/index.js file into their own file, cutting the size of that file by more than half and making it more readable. Now, it is much clearer what each component is responsible for, and what properties are required, instead of having all the logic mixed into one component.

That's another task that can be done in isolation and will make it easier to review the actual changes proposed in this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Query Loop Affects the Query Loop Block
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants