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

Query block: Expose initial templates as block variations #26378

Merged
merged 7 commits into from
Oct 27, 2020

Conversation

ntsekouras
Copy link
Contributor

@ntsekouras ntsekouras commented Oct 22, 2020

Description

Resolves: #26194

This PR expose initial templates like Title and Excerpt, Title and Featured Image as block variations of the Query block.
For now it uses the Placeholder pattern like Columns and these variations are not exposed in the inserter.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

@ntsekouras ntsekouras self-assigned this Oct 22, 2020
@github-actions
Copy link

github-actions bot commented Oct 22, 2020

Size Change: +573 B (0%)

Total Size: 1.2 MB

Filename Size Change
build/block-library/index.js 146 kB +573 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/annotations/index.js 3.53 kB 0 B
build/api-fetch/index.js 3.34 kB 0 B
build/autop/index.js 2.73 kB 0 B
build/blob/index.js 664 B 0 B
build/block-directory/index.js 8.58 kB 0 B
build/block-directory/style-rtl.css 943 B 0 B
build/block-directory/style.css 942 B 0 B
build/block-editor/index.js 130 kB 0 B
build/block-editor/style-rtl.css 11 kB 0 B
build/block-editor/style.css 11 kB 0 B
build/block-library/editor-rtl.css 8.93 kB 0 B
build/block-library/editor.css 8.93 kB 0 B
build/block-library/style-rtl.css 7.75 kB 0 B
build/block-library/style.css 7.75 kB 0 B
build/block-library/theme-rtl.css 741 B 0 B
build/block-library/theme.css 741 B 0 B
build/block-serialization-default-parser/index.js 1.78 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/blocks/index.js 47.6 kB 0 B
build/components/index.js 172 kB 0 B
build/components/style-rtl.css 15.3 kB 0 B
build/components/style.css 15.3 kB 0 B
build/compose/index.js 9.62 kB 0 B
build/core-data/index.js 12.1 kB 0 B
build/data-controls/index.js 679 B 0 B
build/data/index.js 8.61 kB 0 B
build/date/index.js 31.8 kB 0 B
build/deprecated/index.js 769 B 0 B
build/dom-ready/index.js 571 B 0 B
build/dom/index.js 4.42 kB 0 B
build/edit-navigation/index.js 10.7 kB 0 B
build/edit-navigation/style-rtl.css 881 B 0 B
build/edit-navigation/style.css 885 B 0 B
build/edit-post/index.js 306 kB 0 B
build/edit-post/style-rtl.css 6.37 kB 0 B
build/edit-post/style.css 6.35 kB 0 B
build/edit-site/index.js 22.2 kB 0 B
build/edit-site/style-rtl.css 3.79 kB 0 B
build/edit-site/style.css 3.79 kB 0 B
build/edit-widgets/index.js 26.6 kB 0 B
build/edit-widgets/style-rtl.css 3.09 kB 0 B
build/edit-widgets/style.css 3.09 kB 0 B
build/editor/editor-styles-rtl.css 480 B 0 B
build/editor/editor-styles.css 482 B 0 B
build/editor/index.js 42.6 kB 0 B
build/editor/style-rtl.css 3.85 kB 0 B
build/editor/style.css 3.84 kB 0 B
build/element/index.js 4.44 kB 0 B
build/escape-html/index.js 735 B 0 B
build/format-library/index.js 7.47 kB 0 B
build/format-library/style-rtl.css 547 B 0 B
build/format-library/style.css 548 B 0 B
build/hooks/index.js 1.74 kB 0 B
build/html-entities/index.js 623 B 0 B
build/i18n/index.js 3.55 kB 0 B
build/is-shallow-equal/index.js 712 B 0 B
build/keyboard-shortcuts/index.js 2.38 kB 0 B
build/keycodes/index.js 1.84 kB 0 B
build/list-reusable-blocks/index.js 3.01 kB 0 B
build/list-reusable-blocks/style-rtl.css 476 B 0 B
build/list-reusable-blocks/style.css 476 B 0 B
build/media-utils/index.js 5.11 kB 0 B
build/notices/index.js 1.69 kB 0 B
build/nux/index.js 3.26 kB 0 B
build/nux/style-rtl.css 671 B 0 B
build/nux/style.css 668 B 0 B
build/plugins/index.js 2.44 kB 0 B
build/primitives/index.js 1.35 kB 0 B
build/priority-queue/index.js 790 B 0 B
build/redux-routine/index.js 2.85 kB 0 B
build/reusable-blocks/index.js 3.06 kB 0 B
build/rich-text/index.js 13 kB 0 B
build/server-side-render/index.js 2.6 kB 0 B
build/shortcode/index.js 1.69 kB 0 B
build/token-list/index.js 1.24 kB 0 B
build/url/index.js 4.05 kB 0 B
build/viewport/index.js 1.74 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.22 kB 0 B

compressed-size-action

@mapk
Copy link
Contributor

mapk commented Oct 22, 2020

Hey @ntsekouras, thanks for getting this PR underway!

I took a screenshot of what I'm seeing, and sharing it as a visual representation of where this PR is currently. I know the icons are just placeholders right now, so I went ahead and created some of the other SVGs so you can use those instead.

Screenshot

Screen Shot 2020-10-22 at 1 44 10 PM

Title & Date

Screen Shot 2020-10-22 at 1 38 35 PM

SVG

<SVG xmlns="http://www.w3.org/2000/svg" viewBox="0 0 34 30">
  <Path d="M34 0H0v3h34V0zM12 5H0v1h12V5zM0 17h12v1H0v-1zm34-5H0v3h34v-3zM0 29h12v1H0v-1zm34-5H0v3h34v-3z" />
</SVG>

Title & Excerpt

title-excerpt

SVG

<SVG xmlns="http://www.w3.org/2000/svg" viewBox="0 0 34 30">
  <Path d="M34 0H0v3h34V0zm-4 5H0v1h30V5zm4 3H0v1h34V8zM0 11h30v1H0v-1zm0 12h30v1H0v-1zm34 3H0v1h34v-1zM0 29h30v1H0v-1zm34-11H0v3h34v-3z" />
</SVG>

Title, Date & Excerpt

title-date-excerpt

SVG

<SVG xmlns="http://www.w3.org/2000/svg" viewBox="0 0 34 30">
  <Path d="M34 0H0v3h34V0zM12 5H0v1h12V5zm22 3H0v1h34V8zM0 11h34v1H0v-1zm0 12h12v1H0v-1zm34 3H0v1h34v-1zM0 29h34v1H0v-1zm34-11H0v3h34v-3z" />
</SVG>

Title & Content

title-content

SVG

<SVG xmlns="http://www.w3.org/2000/svg" viewBox="0 0 34 30">
  <Path d="M0 5h30v1H0zM0 17h30v1H0zM0 11h30v1H0zM0 23h30v1H0zM0 8h34v1H0zM0 20h34v1H0zM0 14h34v1H0zM0 26h34v1H0zM0 29h34v1H0zM0 0h34v3H0z"/>
</SVG>

@ntsekouras
Copy link
Contributor Author

Thanks @mapk ! I've added them and looks nice :)

Do you think we should make one more variation to start with including Post Featured Image? Just a simple one with
Title, Featured Image in a vertical layout (title and below the image). If yes it would be great if you make an icon as well.

@mtias
Copy link
Member

mtias commented Oct 23, 2020

I really want to include at least one with featured image. We can remove some of the others if necessary.

@ntsekouras ntsekouras marked this pull request as ready for review October 23, 2020 08:40
@mtias
Copy link
Member

mtias commented Oct 23, 2020

I don't think these icons math the general visual flair of our icons super well, but let's merge this as a start anyways. We can also continue to refine the specific templates offered further.

Another thing that comes to mind is that instead of a series of bundles we could offer the user to customize with checkboxes like:

  • Title
  • Date
  • Featured Image
  • Excerpt
  • Content

(This is just a thought to consider and should not block this PR.)

Copy link
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

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

Great one. I'm glad it works without too many hassles by replicating the Columns block. I left some comments but I don't consider them blockers.

I tested new behavior and it all works as expected, including proper initialization after page reload.

@@ -75,4 +76,17 @@ export default function QueryEdit( {
);
}

const QueryEdit = ( props ) => {
const { clientId } = props;
const hasInnerBlocks = useSelect(
Copy link
Member

Choose a reason for hiding this comment

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

I'm surprised it works out of the box when replicated from Columns block. Let's see if this is something that could be further abstracted as proposed in #20582.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I agree with that and saw that some logic might be extracted, but I think it's better for a follow up.

Another thing I wanted to wait before looking into this, is that for Query the Placeholder will be probably be a steps init block procedure. At first select some 'layout(still block variation technically) of the block and then a second step to select some other block options. So I'd like how this could work and be abstracted for other blocks as well.

packages/block-library/src/query/icons.js Outdated Show resolved Hide resolved
packages/e2e-tests/specs/editor/various/autosave.test.js Outdated Show resolved Hide resolved
@mapk
Copy link
Contributor

mapk commented Oct 23, 2020

I'd like to have the icons limited to 34px in width (just like the Columns block) while the button itself remains at 48px before this gets merged. The result should look like this.

Screen Shot 2020-10-23 at 7 52 34 AM

@ntsekouras, I remember you mentioning that the grid formation with the featured image doesn't work right now, so I can get you another icon that just shows the featured image stacked on top of the other text.

@mapk
Copy link
Contributor

mapk commented Oct 23, 2020

@ntsekouras here is an SVG for the featured image option.

Title, Date & Featured Image

Screen Shot 2020-10-23 at 8 02 08 AM

SVG

<SVG xmlns="http://www.w3.org/2000/svg" viewBox="0 0 34 30">
  <Path d="M0 0h34v6H0V0zm12 8H0v1h12V8zm18 3H0v1h30v-1zm0 18H0v1h30v-1zM0 26h12v1H0v-1zm34-8H0v6h34v-6z" />
</SVG>

@mapk
Copy link
Contributor

mapk commented Oct 23, 2020

I don't think these icons math the general visual flair of our icons super well, but let's merge this as a start anyways.

@mtias I'm happy to iterate on icons. Currently, their width should match the Columns icons and be limited to 34px. Both Column layout choices and Query layout choices would then still remain wider than the rest of our icons which are set at 24px, mostly.

@ntsekouras ntsekouras force-pushed the try/query-templates-variations branch 3 times, most recently from 18e9d56 to 9e98251 Compare October 26, 2020 17:49
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.

Expose initial templates as variations
4 participants