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

Fix template part selection searches to use title/area instead of slug/theme. #31520

Merged
merged 6 commits into from
May 11, 2021

Conversation

Addison-Stavlo
Copy link
Contributor

@Addison-Stavlo Addison-Stavlo commented May 5, 2021

Description

Currently, the template part selection search filters based off of slug and theme values.

  • slug is no longer appropriate as all custom template parts have a slug of the form template-part-*, and only the title is editable and visible by the user. Therefore the search input should now work based off of the title instead of the slug.
  • theme is also no longer appropriate as all results are returned for the active theme. Instead, we can replace this with the new area term which will allow both searching by area and label the results such that a user knows if the item corresponds to a different area.

New behavior:

  • Searching by name (title) should now work as expected.
  • With no search input, only results corresponding to the currently active area/variation will be shown.
  • With a search input, results are shown for all areas and are labelled appropriately.

Note that the future of this selection interface is uncertain and may go through redesign or get scrapped altogether, so its probably not worth investing too much time into currently. However, it is important that it at least continues to work with the more recent changes in FSE.

How has this been tested?

Check the following through both the placeholder 'choose' button ( block inserter -> header, footer, or template part) and through the block toolbar's "replace" button.

  • Verify only template parts of the same area are shown by default in the selection component.
  • Verify searching by title works as expected and that search results are labeled by their area.
  • Verify searching by another area works as expected.
  • Smoke test standard template part flows.

Screenshots

Types of changes

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • I've tested my changes with keyboard and screen readers.
  • 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 (please manually search all *.native.js files for terms that need renaming or removal).

@github-actions
Copy link

github-actions bot commented May 5, 2021

Size Change: +2.11 kB (0%)

Total Size: 1.31 MB

Filename Size Change
build/a11y/index.js 1.11 kB -1 B (0%)
build/annotations/index.js 2.93 kB +2 B (0%)
build/api-fetch/index.js 2.42 kB -1 B (0%)
build/blob/index.js 673 B +1 B (0%)
build/block-directory/index.js 6.6 kB -4 B (0%)
build/block-editor/index.js 116 kB +287 B (0%)
build/block-editor/style-rtl.css 13 kB -28 B (0%)
build/block-editor/style.css 13 kB -26 B (0%)
build/block-library/blocks/navigation/editor-rtl.css 1.32 kB +36 B (+3%)
build/block-library/blocks/navigation/editor.css 1.31 kB +36 B (+3%)
build/block-library/blocks/post-comments-form/style-rtl.css 140 B -110 B (-44%) 🎉
build/block-library/blocks/post-comments-form/style.css 140 B -110 B (-44%) 🎉
build/block-library/blocks/post-featured-image/style-rtl.css 119 B +19 B (+19%) ⚠️
build/block-library/blocks/post-featured-image/style.css 119 B +19 B (+19%) ⚠️
build/block-library/editor-rtl.css 9.62 kB +79 B (+1%)
build/block-library/editor.css 9.61 kB +75 B (+1%)
build/block-library/index.js 142 kB +401 B (0%)
build/block-library/style-rtl.css 9.62 kB +121 B (+1%)
build/block-library/style.css 9.63 kB +127 B (+1%)
build/block-serialization-default-parser/index.js 1.3 kB -1 B (0%)
build/blocks/index.js 47.1 kB +72 B (0%)
build/components/index.js 188 kB +1.65 kB (+1%)
build/compose/index.js 9.92 kB -39 B (0%)
build/core-data/index.js 12.1 kB -5 B (0%)
build/customize-widgets/index.js 5.99 kB +170 B (+3%)
build/customize-widgets/style-rtl.css 698 B +9 B (+1%)
build/customize-widgets/style.css 699 B +9 B (+1%)
build/data/index.js 7.22 kB -2 B (0%)
build/date/index.js 31.8 kB -2 B (0%)
build/dom/index.js 4.62 kB -1 B (0%)
build/edit-navigation/index.js 13.6 kB -100 B (-1%)
build/edit-navigation/style-rtl.css 2.83 kB -29 B (-1%)
build/edit-navigation/style.css 2.83 kB -29 B (-1%)
build/edit-post/index.js 333 kB -345 B (0%)
build/edit-post/style-rtl.css 6.79 kB -187 B (-3%)
build/edit-post/style.css 6.78 kB -183 B (-3%)
build/edit-site/index.js 26.1 kB -91 B (0%)
build/edit-site/style-rtl.css 4.79 kB -113 B (-2%)
build/edit-site/style.css 4.78 kB -113 B (-2%)
build/edit-widgets/index.js 12.6 kB +40 B (0%)
build/edit-widgets/style-rtl.css 3.02 kB +45 B (+2%)
build/edit-widgets/style.css 3.03 kB +45 B (+2%)
build/editor/index.js 60.5 kB -1 B (0%)
build/element/index.js 3.44 kB +1 B (0%)
build/i18n/index.js 3.73 kB -1 B (0%)
build/list-reusable-blocks/index.js 2.06 kB -1 B (0%)
build/media-utils/index.js 3.08 kB +1 B (0%)
build/notices/index.js 1.07 kB +2 B (0%)
build/nux/index.js 2.31 kB +3 B (0%)
build/plugins/index.js 2 kB -3 B (0%)
build/primitives/index.js 1.03 kB -6 B (-1%)
build/reusable-blocks/index.js 2.56 kB +2 B (0%)
build/rich-text/index.js 12.2 kB +391 B (+3%)
build/server-side-render/index.js 1.64 kB -2 B (0%)
build/shortcode/index.js 1.68 kB -1 B (0%)
build/token-list/index.js 847 B -1 B (0%)
build/viewport/index.js 1.28 kB -2 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/autop/index.js 2.28 kB 0 B
build/block-directory/style-rtl.css 993 B 0 B
build/block-directory/style.css 995 B 0 B
build/block-library/blocks/archives/editor-rtl.css 61 B 0 B
build/block-library/blocks/archives/editor.css 60 B 0 B
build/block-library/blocks/audio/editor-rtl.css 58 B 0 B
build/block-library/blocks/audio/editor.css 58 B 0 B
build/block-library/blocks/audio/style-rtl.css 112 B 0 B
build/block-library/blocks/audio/style.css 112 B 0 B
build/block-library/blocks/block/editor-rtl.css 161 B 0 B
build/block-library/blocks/block/editor.css 161 B 0 B
build/block-library/blocks/button/editor-rtl.css 475 B 0 B
build/block-library/blocks/button/editor.css 474 B 0 B
build/block-library/blocks/button/style-rtl.css 515 B 0 B
build/block-library/blocks/button/style.css 515 B 0 B
build/block-library/blocks/buttons/editor-rtl.css 315 B 0 B
build/block-library/blocks/buttons/editor.css 315 B 0 B
build/block-library/blocks/buttons/style-rtl.css 368 B 0 B
build/block-library/blocks/buttons/style.css 368 B 0 B
build/block-library/blocks/calendar/style-rtl.css 208 B 0 B
build/block-library/blocks/calendar/style.css 208 B 0 B
build/block-library/blocks/categories/editor-rtl.css 84 B 0 B
build/block-library/blocks/categories/editor.css 83 B 0 B
build/block-library/blocks/categories/style-rtl.css 79 B 0 B
build/block-library/blocks/categories/style.css 79 B 0 B
build/block-library/blocks/code/style-rtl.css 90 B 0 B
build/block-library/blocks/code/style.css 90 B 0 B
build/block-library/blocks/columns/editor-rtl.css 190 B 0 B
build/block-library/blocks/columns/editor.css 190 B 0 B
build/block-library/blocks/columns/style-rtl.css 422 B 0 B
build/block-library/blocks/columns/style.css 422 B 0 B
build/block-library/blocks/cover/editor-rtl.css 603 B 0 B
build/block-library/blocks/cover/editor.css 604 B 0 B
build/block-library/blocks/cover/style-rtl.css 1.22 kB 0 B
build/block-library/blocks/cover/style.css 1.22 kB 0 B
build/block-library/blocks/embed/editor-rtl.css 486 B 0 B
build/block-library/blocks/embed/editor.css 486 B 0 B
build/block-library/blocks/embed/style-rtl.css 401 B 0 B
build/block-library/blocks/embed/style.css 400 B 0 B
build/block-library/blocks/file/editor-rtl.css 301 B 0 B
build/block-library/blocks/file/editor.css 300 B 0 B
build/block-library/blocks/file/frontend.js 773 B 0 B
build/block-library/blocks/file/style-rtl.css 255 B 0 B
build/block-library/blocks/file/style.css 255 B 0 B
build/block-library/blocks/freeform/editor-rtl.css 2.45 kB 0 B
build/block-library/blocks/freeform/editor.css 2.45 kB 0 B
build/block-library/blocks/gallery/editor-rtl.css 704 B 0 B
build/block-library/blocks/gallery/editor.css 705 B 0 B
build/block-library/blocks/gallery/style-rtl.css 1.06 kB 0 B
build/block-library/blocks/gallery/style.css 1.05 kB 0 B
build/block-library/blocks/group/editor-rtl.css 160 B 0 B
build/block-library/blocks/group/editor.css 160 B 0 B
build/block-library/blocks/group/style-rtl.css 57 B 0 B
build/block-library/blocks/group/style.css 57 B 0 B
build/block-library/blocks/heading/editor-rtl.css 129 B 0 B
build/block-library/blocks/heading/editor.css 129 B 0 B
build/block-library/blocks/heading/style-rtl.css 76 B 0 B
build/block-library/blocks/heading/style.css 76 B 0 B
build/block-library/blocks/html/editor-rtl.css 281 B 0 B
build/block-library/blocks/html/editor.css 281 B 0 B
build/block-library/blocks/image/editor-rtl.css 717 B 0 B
build/block-library/blocks/image/editor.css 716 B 0 B
build/block-library/blocks/image/style-rtl.css 476 B 0 B
build/block-library/blocks/image/style.css 478 B 0 B
build/block-library/blocks/latest-comments/style-rtl.css 281 B 0 B
build/block-library/blocks/latest-comments/style.css 282 B 0 B
build/block-library/blocks/latest-posts/editor-rtl.css 137 B 0 B
build/block-library/blocks/latest-posts/editor.css 137 B 0 B
build/block-library/blocks/latest-posts/style-rtl.css 523 B 0 B
build/block-library/blocks/latest-posts/style.css 522 B 0 B
build/block-library/blocks/legacy-widget/editor-rtl.css 557 B 0 B
build/block-library/blocks/legacy-widget/editor.css 557 B 0 B
build/block-library/blocks/list/style-rtl.css 63 B 0 B
build/block-library/blocks/list/style.css 63 B 0 B
build/block-library/blocks/media-text/editor-rtl.css 176 B 0 B
build/block-library/blocks/media-text/editor.css 176 B 0 B
build/block-library/blocks/media-text/style-rtl.css 492 B 0 B
build/block-library/blocks/media-text/style.css 489 B 0 B
build/block-library/blocks/more/editor-rtl.css 434 B 0 B
build/block-library/blocks/more/editor.css 434 B 0 B
build/block-library/blocks/navigation-link/editor-rtl.css 617 B 0 B
build/block-library/blocks/navigation-link/editor.css 619 B 0 B
build/block-library/blocks/navigation-link/style-rtl.css 94 B 0 B
build/block-library/blocks/navigation-link/style.css 94 B 0 B
build/block-library/blocks/navigation/style-rtl.css 1.27 kB 0 B
build/block-library/blocks/navigation/style.css 1.27 kB 0 B
build/block-library/blocks/nextpage/editor-rtl.css 395 B 0 B
build/block-library/blocks/nextpage/editor.css 395 B 0 B
build/block-library/blocks/page-list/editor-rtl.css 239 B 0 B
build/block-library/blocks/page-list/editor.css 240 B 0 B
build/block-library/blocks/page-list/style-rtl.css 167 B 0 B
build/block-library/blocks/page-list/style.css 167 B 0 B
build/block-library/blocks/paragraph/editor-rtl.css 157 B 0 B
build/block-library/blocks/paragraph/editor.css 157 B 0 B
build/block-library/blocks/paragraph/style-rtl.css 247 B 0 B
build/block-library/blocks/paragraph/style.css 248 B 0 B
build/block-library/blocks/post-author/editor-rtl.css 209 B 0 B
build/block-library/blocks/post-author/editor.css 209 B 0 B
build/block-library/blocks/post-author/style-rtl.css 183 B 0 B
build/block-library/blocks/post-author/style.css 184 B 0 B
build/block-library/blocks/post-comments/style-rtl.css 362 B 0 B
build/block-library/blocks/post-comments/style.css 362 B 0 B
build/block-library/blocks/post-content/editor-rtl.css 139 B 0 B
build/block-library/blocks/post-content/editor.css 139 B 0 B
build/block-library/blocks/post-excerpt/editor-rtl.css 73 B 0 B
build/block-library/blocks/post-excerpt/editor.css 73 B 0 B
build/block-library/blocks/post-excerpt/style-rtl.css 69 B 0 B
build/block-library/blocks/post-excerpt/style.css 69 B 0 B
build/block-library/blocks/post-featured-image/editor-rtl.css 338 B 0 B
build/block-library/blocks/post-featured-image/editor.css 338 B 0 B
build/block-library/blocks/post-title/style-rtl.css 60 B 0 B
build/block-library/blocks/post-title/style.css 60 B 0 B
build/block-library/blocks/preformatted/style-rtl.css 103 B 0 B
build/block-library/blocks/preformatted/style.css 103 B 0 B
build/block-library/blocks/pullquote/editor-rtl.css 183 B 0 B
build/block-library/blocks/pullquote/editor.css 183 B 0 B
build/block-library/blocks/pullquote/style-rtl.css 318 B 0 B
build/block-library/blocks/pullquote/style.css 318 B 0 B
build/block-library/blocks/query-loop/editor-rtl.css 83 B 0 B
build/block-library/blocks/query-loop/editor.css 82 B 0 B
build/block-library/blocks/query-loop/style-rtl.css 315 B 0 B
build/block-library/blocks/query-loop/style.css 317 B 0 B
build/block-library/blocks/query-pagination-numbers/editor-rtl.css 122 B 0 B
build/block-library/blocks/query-pagination-numbers/editor.css 121 B 0 B
build/block-library/blocks/query-pagination/editor-rtl.css 270 B 0 B
build/block-library/blocks/query-pagination/editor.css 262 B 0 B
build/block-library/blocks/query-pagination/style-rtl.css 168 B 0 B
build/block-library/blocks/query-pagination/style.css 168 B 0 B
build/block-library/blocks/query-title/editor-rtl.css 86 B 0 B
build/block-library/blocks/query-title/editor.css 86 B 0 B
build/block-library/blocks/query/editor-rtl.css 131 B 0 B
build/block-library/blocks/query/editor.css 132 B 0 B
build/block-library/blocks/quote/style-rtl.css 169 B 0 B
build/block-library/blocks/quote/style.css 169 B 0 B
build/block-library/blocks/rss/editor-rtl.css 201 B 0 B
build/block-library/blocks/rss/editor.css 202 B 0 B
build/block-library/blocks/rss/style-rtl.css 290 B 0 B
build/block-library/blocks/rss/style.css 290 B 0 B
build/block-library/blocks/search/editor-rtl.css 189 B 0 B
build/block-library/blocks/search/editor.css 189 B 0 B
build/block-library/blocks/search/style-rtl.css 359 B 0 B
build/block-library/blocks/search/style.css 362 B 0 B
build/block-library/blocks/separator/editor-rtl.css 99 B 0 B
build/block-library/blocks/separator/editor.css 99 B 0 B
build/block-library/blocks/separator/style-rtl.css 251 B 0 B
build/block-library/blocks/separator/style.css 251 B 0 B
build/block-library/blocks/shortcode/editor-rtl.css 512 B 0 B
build/block-library/blocks/shortcode/editor.css 512 B 0 B
build/block-library/blocks/site-logo/editor-rtl.css 440 B 0 B
build/block-library/blocks/site-logo/editor.css 441 B 0 B
build/block-library/blocks/site-logo/style-rtl.css 154 B 0 B
build/block-library/blocks/site-logo/style.css 154 B 0 B
build/block-library/blocks/social-link/editor-rtl.css 164 B 0 B
build/block-library/blocks/social-link/editor.css 165 B 0 B
build/block-library/blocks/social-links/editor-rtl.css 796 B 0 B
build/block-library/blocks/social-links/editor.css 795 B 0 B
build/block-library/blocks/social-links/style-rtl.css 1.32 kB 0 B
build/block-library/blocks/social-links/style.css 1.33 kB 0 B
build/block-library/blocks/spacer/editor-rtl.css 308 B 0 B
build/block-library/blocks/spacer/editor.css 308 B 0 B
build/block-library/blocks/spacer/style-rtl.css 48 B 0 B
build/block-library/blocks/spacer/style.css 48 B 0 B
build/block-library/blocks/table/editor-rtl.css 478 B 0 B
build/block-library/blocks/table/editor.css 478 B 0 B
build/block-library/blocks/table/style-rtl.css 485 B 0 B
build/block-library/blocks/table/style.css 485 B 0 B
build/block-library/blocks/tag-cloud/editor-rtl.css 118 B 0 B
build/block-library/blocks/tag-cloud/editor.css 118 B 0 B
build/block-library/blocks/tag-cloud/style-rtl.css 94 B 0 B
build/block-library/blocks/tag-cloud/style.css 94 B 0 B
build/block-library/blocks/template-part/editor-rtl.css 551 B 0 B
build/block-library/blocks/template-part/editor.css 550 B 0 B
build/block-library/blocks/term-description/editor-rtl.css 90 B 0 B
build/block-library/blocks/term-description/editor.css 90 B 0 B
build/block-library/blocks/text-columns/editor-rtl.css 95 B 0 B
build/block-library/blocks/text-columns/editor.css 95 B 0 B
build/block-library/blocks/text-columns/style-rtl.css 166 B 0 B
build/block-library/blocks/text-columns/style.css 166 B 0 B
build/block-library/blocks/verse/style-rtl.css 87 B 0 B
build/block-library/blocks/verse/style.css 87 B 0 B
build/block-library/blocks/video/editor-rtl.css 569 B 0 B
build/block-library/blocks/video/editor.css 570 B 0 B
build/block-library/blocks/video/style-rtl.css 169 B 0 B
build/block-library/blocks/video/style.css 169 B 0 B
build/block-library/common-rtl.css 1.26 kB 0 B
build/block-library/common.css 1.26 kB 0 B
build/block-library/reset-rtl.css 506 B 0 B
build/block-library/reset.css 507 B 0 B
build/block-library/theme-rtl.css 692 B 0 B
build/block-library/theme.css 693 B 0 B
build/block-serialization-spec-parser/index.js 3.06 kB 0 B
build/components/style-rtl.css 16.2 kB 0 B
build/components/style.css 16.2 kB 0 B
build/data-controls/index.js 830 B 0 B
build/deprecated/index.js 738 B 0 B
build/dom-ready/index.js 576 B 0 B
build/edit-post/classic-rtl.css 454 B 0 B
build/edit-post/classic.css 454 B 0 B
build/editor/style-rtl.css 3.95 kB 0 B
build/editor/style.css 3.95 kB 0 B
build/escape-html/index.js 739 B 0 B
build/format-library/index.js 5.68 kB 0 B
build/format-library/style-rtl.css 637 B 0 B
build/format-library/style.css 639 B 0 B
build/hooks/index.js 1.76 kB 0 B
build/html-entities/index.js 628 B 0 B
build/is-shallow-equal/index.js 710 B 0 B
build/keyboard-shortcuts/index.js 1.65 kB 0 B
build/keycodes/index.js 1.43 kB 0 B
build/list-reusable-blocks/style-rtl.css 629 B 0 B
build/list-reusable-blocks/style.css 628 B 0 B
build/nux/style-rtl.css 718 B 0 B
build/nux/style.css 716 B 0 B
build/priority-queue/index.js 791 B 0 B
build/react-i18n/index.js 924 B 0 B
build/redux-routine/index.js 2.82 kB 0 B
build/reusable-blocks/style-rtl.css 225 B 0 B
build/reusable-blocks/style.css 225 B 0 B
build/url/index.js 1.95 kB 0 B
build/warning/index.js 1.13 kB 0 B
build/widgets/index.js 1.68 kB 0 B
build/wordcount/index.js 1.24 kB 0 B

compressed-size-action

@Addison-Stavlo Addison-Stavlo self-assigned this May 5, 2021
@Addison-Stavlo Addison-Stavlo changed the title Update template part selection interface to use title/area instead of slug/theme. Fix template part selection searches to use title/area instead of slug/theme. May 6, 2021
@Addison-Stavlo Addison-Stavlo added [Block] Template Part Affects the Template Parts Block [Feature] Full Site Editing [Type] Bug An existing feature does not function as intended labels May 6, 2021
@Addison-Stavlo Addison-Stavlo marked this pull request as ready for review May 6, 2021 13:49
@Addison-Stavlo Addison-Stavlo requested a review from ajitbohra as a code owner May 6, 2021 13:49
@jameskoster
Copy link
Contributor

Possible bug: I'm not seeing any TPs until I search:

tp

@Addison-Stavlo
Copy link
Contributor Author

Possible bug: I'm not seeing any TPs until I search:

Are you on the latest commit? (398fd8e) - I noticed this as well from the general placeholder and that most recent commit should have fixed it.

@jameskoster
Copy link
Contributor

Fixed :D

@david-szabo97
Copy link
Member

It's acting a bit weird for me 🤔 Huge empty spaces and it only shows a single template part when the search input is empty.

Screen.Recording.2021-05-07.at.11.06.59.mov

@Addison-Stavlo
Copy link
Contributor Author

Addison-Stavlo commented May 7, 2021

Huge empty spaces

Im not sure I follow?

it only shows a single template part when the search input is empty.

I assume this is because you inserted the general/uncategorized placeholder ("Template Part" in the inserter). This would then only show other template parts of the same area (general/uncat) before typing in the search input. So you are seeing your 1 template part that is of that area, which seems to have no content. Try inserting a "Header" or "Footer" variation block from the inserter, it should show those variations instead when opened.

@Addison-Stavlo
Copy link
Contributor Author

Huge empty spaces
Im not sure I follow?

Oh, where the previews should be that disappear after a split second? I think thats a result of the useAsyncList implementation that renders placeholders (the blank items) while it loads in the previews in. That way the previews populate in an ordered manner as opposed to all trying to load in at the same time making a very jumpy and unordered experience. I don't think this PR has changed anything with that.

it only shows a single template part when the search input is empty.
I assume this is because you inserted the general/uncategorized placeholder ("Template Part" in the inserter).

I wonder if the general/uncategorized case should show ALL areas by default instead of just other general/uncategorized ones. Maybe in just the placeholder state and not in the established block state? or both? Any thoughts on this @jameskoster ?

@jameskoster
Copy link
Contributor

Hmm, yes I think my expectation would be that a "Template Part" block would act as a base layer and provide access to all template parts as a default.

So in the case of something like a Header, I could insert the "Header" block directly, or insert the Template Part block and choose Header.

@Addison-Stavlo
Copy link
Contributor Author

yes I think my expectation would be that a "Template Part" block would act as a base layer and provide access to all template parts as a default.

Makes sense to me! Updated.

@david-szabo97
Copy link
Member

With the latest changes, it feels a lot better!

Oh, where the previews should be that disappear after a split second? I think thats a result of the useAsyncList implementation that renders placeholders (the blank items) while it loads in the previews in. That way the previews populate in an ordered manner as opposed to all trying to load in at the same time making a very jumpy and unordered experience. I don't think this PR has changed anything with that.

Gotcha, it's not a pleasant experience. Hopefully we can fix this somehow in the future.


I'm not quite sure it's related, but I'd expect the Replace to not show (or disabled) the current template part in the list.

This is the header template part. I click on the Replace button and it shows the header template part again. This action doesn't have any value in this case. @jameskoster thoughts? (Note: I only have a single header template part.)

image

@Addison-Stavlo
Copy link
Contributor Author

With the latest changes, it feels a lot better!

Awesome!

Gotcha, it's not a pleasant experience. Hopefully we can fix this somehow in the future.

Yeah, im not sure the solution in this specific circumstance. Maybe showing gray boxes instead of white space will help visually, although it will still be jumpy as they load in as we can't accurately predict the height of the preview before its loaded. 🤔 Moving this flow to a different type of component such as a modal or something else with fixed height may feel better. I would expect we need to re-design this interaction and UI overall at some point.

I'm not quite sure it's related, but I'd expect the Replace to not show (or disabled) the current template part in the list.

Good point. It has always been like this since its really the 'selection' component being used in a replacement context with no special case for it. Im not sure the best solution design wise. Not showing any results may feel odd, and showing a result that is disabled/not clickable might be a bit odd as well? I guess its 'kind of' disabled in that if you do select it nothing really happens.

@jameskoster
Copy link
Contributor

Good spot @david-szabo97, I agree that the selected template part should not appear in the replacement list at all. And if there are no replacement TPs available, the "Replace" button itself should also be hidden.

@Addison-Stavlo
Copy link
Contributor Author

@jameskoster - 👍 Il open a follow up for these: limiting this PR to more of a bug fix for the search input and the follow up to polishing its replace functionality a bit more.

I think we can hide the currently used template part from the results. However, I have some concerns about disabling the 'replace' button (explained more below). Now, the combination of these could result in the user opening 'replace' to see zero options by default (but have the ability to search others via the input). Maybe the zero default results isn't an issue if we label things properly? 🤔

Hiding 'replace' if there are no other areas of that type: This will restrict capabilities since folks are able to use the search input to find template parts of any type. Imagine the user created a template part they wanted to be a header, but forgot to assign its area so it is still general. They then go to a template and try to replace the current header with what they created. They select 'replace' and only see the default header option, but see a search input and can type in the name of the header they had made. They see its labeled under "General", can select it, and then know they need to assign its area. However, if the replace button is greyed out.... 🤷‍♀️ maybe they are a new user and still need to 'discover' the various template part variation blocks to insert or somehow figure out that they need to go back to that template part they created and assign it as a 'header'. The process of doing so could end up a quite a speed bump taking energy away from what they really want to do.

Stepping back - what are your thoughts on this component in general? Does it seem like it needs a re-design at some point overall? I think it was tossed together about a year ago when FSE and template parts were a bit different, with little (or no) design input. It may make sense to rethink it from the ground up at some point?

@Addison-Stavlo Addison-Stavlo merged commit cb9fd48 into trunk May 11, 2021
@Addison-Stavlo Addison-Stavlo deleted the try/update-template-part-selection-for-area branch May 11, 2021 12:55
@github-actions github-actions bot added this to the Gutenberg 10.7 milestone May 11, 2021
@jameskoster
Copy link
Contributor

Stepping back - what are your thoughts on this component in general? Does it seem like it needs a re-design at some point overall?

Good question. At a high level I'm inclined to say that easy swapping of header and footer TP variations seems sensible. IE swap "Header A" for "Header B", or swap "Footer 1" for "Footer 2".

But since the scope for general template parts is so broad, the swapping behaviour arguably loses a lot of value. So perhaps it shouldn't be as prominent in the UI for those template parts, or maybe it shouldn't be in the UI at all.

That brings us to searching... it seems unlikely to me that a given site will have so many headers or footers that searching will be useful in any practical way. And if we say that general TPs cannot be swapped, then we probably don't need a search input in the swap interface for headers and footers.

All said and done, it is probably best if we can concentrate search activities around the existing affordances, IE the search input in the Inspector, and the slash command quick inserter.

@Addison-Stavlo
Copy link
Contributor Author

Yeah, while the search input can be useful for me at times (ive created TONs of blank or converted TPs just from PR testing), its unlikely that a user will have that many (but who knows? 😆 ). Id expect the end state of this component may not need that.

And also remember, this component is not only about 'replacement'. Its also about 'selection' when we insert a Header/Footer/General block. Id expect the desired flow in the end would be something a bit different than a dropdown with a search input (whether thats finding the desired template part in the inserter somehow, or having a different component than this to preview/select.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Template Part Affects the Template Parts Block [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants