Query Loop block: Convert the post content type setting to a ToggleGroupControl if there are few items #63375
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
priethor
added
[Type] Enhancement
|
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: +64 B (0%) Total Size: 1.75 MB
ℹ️ View Unchanged
|
|
Have you tested with third-party post types available as well? How well does this scale? |
|
I have tested it with the third-party post types provided by the |
youknowriad
reviewed
Jul 11, 2024
packages/block-library/src/query/edit/inspector-controls/index.js
Outdated
Show resolved
Hide resolved
youknowriad
reviewed
Jul 11, 2024
packages/block-library/src/query/edit/inspector-controls/index.js
Outdated
Show resolved
Hide resolved
|
This is looking good to me. The spacing is a bit off maybe so let's see what designers say. |
|
Another quick check: would a ToggleGroup work better here? If it's only ever for two items max? |
|
ToggleGroup would be nice if it were two, but we don't want many edgecases, i.e. togglegroup, then radios, then dropdown. Radios work okay as far as translatability of long strings, though. So if the PR is mainly to optimize for the case of "Post" and "Page", I'd go segmented control as Rich suggests: it seems to be the 80% case, and the specific design of the segmented control better affords the very verbose help-text that sits right below the box. |
Since built-in post types cannot be unregistered via |
|
Here's an attempt with a Grabacion.de.pantalla.2024-07-12.a.las.12.06.57.mov |
|
Looks good. Can you make the segmented control full width? I.e. 100% width of the inspector panel. |
|
Gotcha. Even better 👍 Grabacion.de.pantalla.2024-07-12.a.las.12.33.30.movThe "Sticky posts" dropdown has a fixed number of three items. Should I follow up later by converting it to a ToggleControlGroup as well for consistency? |
|
If we believe the 3 items can fit in a segmented control even in translations, definitely a good choice. Otherwise I'd probably keep it a dropdown. "Only" is a bit of a weird term, though. Separately, we can add the |
|
Applied
|
|
Nice work! |
priethor
changed the title
|
Flaky tests detected in 67cfd4e. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/9907035736
|
youknowriad
reviewed
Jul 12, 2024
packages/block-library/src/query/edit/inspector-controls/index.js
Outdated
Show resolved
Hide resolved
youknowriad
approved these changes
Jul 12, 2024
I wold prefer not to add such a feature to In my opinion, low-level components should provide a focused, flexible, and modular primitive, thus allowing some powerful composition patterns. Composing those puzzle pieces together should be done externally by the consumer of the package (cc @WordPress/gutenberg-components ) |
priethor
force-pushed
the
fix/query-loop-post-type-control
branch
from
eb354b6 to
4c871cd
Compare
carstingaxion
pushed a commit
to carstingaxion/gutenberg
that referenced
this pull request
Jul 18, 2024
…oupControl if there are few items (WordPress#63375) Co-authored-by: priethor <priethor@git.wordpress.org> Co-authored-by: youknowriad <youknowriad@git.wordpress.org> Co-authored-by: richtabor <richtabor@git.wordpress.org> Co-authored-by: jasmussen <joen@git.wordpress.org>
14 tasks
11 tasks
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.
Closes #63374.
What?
When there are fewer than three items, this PR converts the Query Loop's post content type inspector control to a
ToggleGroupControlinstead of using theSelectControl.It also changes the label copy from
Post typetoPost contentfor accuracy and applies__next40pxDefaultSizeto this and other Query Loop inspector controls.Why?
Select controls are useful for avoiding visual clutter when there are many options to choose from. However, they require an extra click and do not offer visual improvements when there are few items.
How?
By using a
ToggleGroupControlinstead of aSelectControlwhen there are fewer than three items.Testing Instructions
Inherit query from templatesetting.Post contentoption will appear using a ToggleGroupControl withPostandPageas options.Screenshots or screencast
Grabacion.de.pantalla.2024-07-10.a.las.19.18.10.mov