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 order/order by support in Query block #24691

Merged
merged 5 commits into from
Sep 1, 2020

Conversation

ntsekouras
Copy link
Contributor

@ntsekouras ntsekouras commented Aug 20, 2020

Description

This is a part of #24521 whose ultimate goal is to make Latest posts block a block variation of Query block.

This PR adds order and orderby (date + title) support for the Query block. This functionality is imported from QueryControls which is a component tightly coupled with query filters and seems to be a good place to build upon.

There will also be follow up PRs that will enhance the Query block and QueryControls component so as to support more options from the API.

Although the previous options for Query block where in the block toolbar, I placed the new sorting addition is in the block inspector controls. This seemed to be a better place for adding a lot more options ( to me :) ) as it would be really crowded to add so many things in the toolbar. Of course this could be decided and kept or changed later by just moving these components.

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 added the [Block] Query Loop Affects the Query Loop Block label Aug 20, 2020
@ntsekouras ntsekouras self-assigned this Aug 20, 2020
@github-actions
Copy link

github-actions bot commented Aug 20, 2020

Size Change: +143 B (0%)

Total Size: 1.17 MB

Filename Size Change
build/block-library/index.js 136 kB +143 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/annotations/index.js 3.67 kB 0 B
build/api-fetch/index.js 3.44 kB 0 B
build/autop/index.js 2.82 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/index.js 7.99 kB 0 B
build/block-directory/style-rtl.css 953 B 0 B
build/block-directory/style.css 952 B 0 B
build/block-editor/index.js 126 kB 0 B
build/block-editor/style-rtl.css 10.8 kB 0 B
build/block-editor/style.css 10.8 kB 0 B
build/block-library/editor-rtl.css 8.55 kB 0 B
build/block-library/editor.css 8.55 kB 0 B
build/block-library/style-rtl.css 7.47 kB 0 B
build/block-library/style.css 7.47 kB 0 B
build/block-library/theme-rtl.css 741 B 0 B
build/block-library/theme.css 742 B 0 B
build/block-serialization-default-parser/index.js 1.88 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/blocks/index.js 47.7 kB 0 B
build/components/index.js 200 kB 0 B
build/components/style-rtl.css 15.5 kB 0 B
build/components/style.css 15.5 kB 0 B
build/compose/index.js 9.67 kB 0 B
build/core-data/index.js 12.3 kB 0 B
build/data-controls/index.js 1.29 kB 0 B
build/data/index.js 8.55 kB 0 B
build/date/index.js 5.38 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 568 B 0 B
build/dom/index.js 4.48 kB 0 B
build/edit-navigation/index.js 11.6 kB 0 B
build/edit-navigation/style-rtl.css 1.16 kB 0 B
build/edit-navigation/style.css 1.16 kB 0 B
build/edit-post/index.js 304 kB 0 B
build/edit-post/style-rtl.css 5.61 kB 0 B
build/edit-post/style.css 5.61 kB 0 B
build/edit-site/index.js 17 kB 0 B
build/edit-site/style-rtl.css 3.06 kB 0 B
build/edit-site/style.css 3.06 kB 0 B
build/edit-widgets/index.js 12 kB 0 B
build/edit-widgets/style-rtl.css 2.46 kB 0 B
build/edit-widgets/style.css 2.45 kB 0 B
build/editor/editor-styles-rtl.css 492 B 0 B
build/editor/editor-styles.css 493 B 0 B
build/editor/index.js 45.3 kB 0 B
build/editor/style-rtl.css 3.81 kB 0 B
build/editor/style.css 3.81 kB 0 B
build/element/index.js 4.65 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 7.71 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 2.13 kB 0 B
build/html-entities/index.js 621 B 0 B
build/i18n/index.js 3.57 kB 0 B
build/is-shallow-equal/index.js 710 B 0 B
build/keyboard-shortcuts/index.js 2.52 kB 0 B
build/keycodes/index.js 1.94 kB 0 B
build/list-reusable-blocks/index.js 3.12 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.32 kB 0 B
build/notices/index.js 1.79 kB 0 B
build/nux/index.js 3.4 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.56 kB 0 B
build/primitives/index.js 1.41 kB 0 B
build/priority-queue/index.js 789 B 0 B
build/redux-routine/index.js 2.85 kB 0 B
build/rich-text/index.js 13.9 kB 0 B
build/server-side-render/index.js 2.77 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.27 kB 0 B
build/url/index.js 4.06 kB 0 B
build/viewport/index.js 1.85 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action

@ntsekouras ntsekouras added the [Type] Enhancement A suggestion for improvement. label Aug 21, 2020
@ntsekouras ntsekouras marked this pull request as ready for review August 21, 2020 16:41
@ntsekouras ntsekouras changed the title WIP: Add order/order by support in Query block Add order/order by support in Query block Aug 21, 2020
@draganescu draganescu added Needs Design Needs design efforts. Needs Design Feedback Needs general design feedback. labels Aug 22, 2020
Copy link
Contributor

@draganescu draganescu left a comment

Choose a reason for hiding this comment

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

Great work @ntsekouras ! I tested this and it works. I found it to be a bit slow to update locally, plus the transitions are a bit jarring when the query is changed. This is out of the scope of this PR, but I wanted to mention b/c I added two design labels.

I added "Needs design" because the whole query block could use more direction, but particulaly, and why I added "Needs design feedback" I wonder this:

  • does the toolbar filtering provide a better experience (with a closed inspector) in the context of the site editor where any screen real estate allows content to display better (e.g. sidebars not to collapse because of the inspector)?

If this is the case perhaps we could keep the filtering in the toolbar and polish that interaction more?

@ntsekouras
Copy link
Contributor Author

ntsekouras commented Aug 24, 2020

Thanks for looking into this @draganescu!

I added "Needs design" because the whole query block could use more direction

does the toolbar filtering provide a better experience (with a closed inspector) in the context of the site editor where any screen real estate allows content to display better (e.g. sidebars not to collapse because of the inspector)?

This is true and at first I had put sorting in the toolbar, but since it's just the first step of Query block's enhancements and its ultimate goal is to support way more options from the API, the Query block would be too crowded I think.

I have the impression that toolbar is right for the most common/used settings and the sidebar is for a bit more advanced options. So with the other options to be added it would be good to discuss which are these actions to be in the toolbar and which are to be placed in sidebar.

Also I haven't dig into how the display options would be integrated in the Query block and if they should in the first place ( as they are not related to Query itself as functionality...). Maybe it could be a new displayOptions property? I'm not really sure yet..

Having said that, the change of this specific sorting functionality can be easily moved back in toolbar and make one step forward to the enhancement we want, but we have to keep in mind that the next and really easy to implement step would be author filtering that would add one more row to either the toolbar or the sidebar.

--cc @jasmussen @kjellr any thoughts?

@ntsekouras ntsekouras mentioned this pull request Aug 24, 2020
26 tasks
@jasmussen
Copy link
Contributor

I took a quick look, and alas I've been out of the query loop (if you'll forgive the pun) — how do I best test this? (Here's my failed GIF exploration).

High level I do think I agree that we need some high level design work on the query block. I wonder if it could be a good candidate for @ItsJonQ's component work? But I also wonder: shouldn't that be a separate issue to discuss on, rather than hold up this PR?

@mtias
Copy link
Member

mtias commented Aug 26, 2020

But I also wonder: shouldn't that be a separate issue to discuss on, rather than hold up this PR?

Yes, there's a tracking issue in #24762 to organize the query block work. It'd be good to start by identifying the parameters to support from WP_Query and map it to a UI. Then we can ran that through @ItsJonQ explorations and ensure we are covering all that is needed.

@ntsekouras
Copy link
Contributor Author

But I also wonder: shouldn't that be a separate issue to discuss on, rather than hold up this PR?

It'd be good to start by identifying the parameters to support from WP_Query and map it to a UI.

I agree that we should identify the parameters to support from WP_Query, but this is definitely going to be included (order/orderby title and date).

The implemented functionality can be easily moved later with the design decisions, but I think this is a small improvement from what is shipped now, no?

What do you think? @mtias @draganescu

@mtias
Copy link
Member

mtias commented Aug 31, 2020

Yes, deifnitely!

@ntsekouras ntsekouras requested a review from draganescu August 31, 2020 10:32
@ntsekouras ntsekouras force-pushed the add/order-support-query-block branch from d0abb27 to 3fc613a Compare September 1, 2020 07:41
Copy link
Contributor

@draganescu draganescu left a comment

Choose a reason for hiding this comment

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

I tested this and while it still requires some decision on where to place all the filters and other new settings it works all right and it is a step forward. Code-wise it follows the structure of the rest of the block so 🚢 it!

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 Needs Design Feedback Needs general design feedback. Needs Design Needs design efforts. [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants