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

Navigation: Add Post, Page, Category and Tag variations to Link #24670

Merged
merged 7 commits into from
Aug 26, 2020

Conversation

noisysocks
Copy link
Member

@noisysocks noisysocks commented Aug 20, 2020

Closes #22919. Requires #22600. Split out from #24613.

nav-variations

Adds Post, Page, Category and Tag variations to the Navigation Link block. Each variation sets the type attribute which in turn causes LinkControl to filter its results using the /wp/v2/search endpoint's type and subtype params.

This PR requires #22600 which adds the ability to search for categories and triages using the search endpoint. Do not merge this PR until #22600 is merged and this PR's base is updated.

How to test

  1. Ensure that your site has some posts, pages, categories and tags.
  2. Create a Navigation block.
  3. Add a Link block. You should be able to search for posts, pages, categories and tags.
  4. Add a Page block. You should only be able to search for posts.
  5. ...and so on for Post, Category and Tag.

@noisysocks noisysocks added the [Block] Navigation Affects the Navigation Block label Aug 20, 2020
@noisysocks noisysocks added the Needs Design Feedback Needs general design feedback. label Aug 20, 2020
@noisysocks
Copy link
Member Author

@shaunandrews: Does this flow look right to you?

@github-actions
Copy link

github-actions bot commented Aug 20, 2020

Size Change: +1.99 kB (0%)

Total Size: 1.16 MB

Filename Size Change
build/annotations/index.js 3.67 kB +1 B
build/block-directory/index.js 7.99 kB +21 B (0%)
build/block-editor/index.js 126 kB +173 B (0%)
build/block-library/editor-rtl.css 8.5 kB +4 B (0%)
build/block-library/editor.css 8.5 kB +3 B (0%)
build/block-library/index.js 135 kB +1.68 kB (1%)
build/block-library/style-rtl.css 7.45 kB +23 B (0%)
build/block-library/style.css 7.45 kB +21 B (0%)
build/components/index.js 200 kB +4 B (0%)
build/core-data/index.js 12.3 kB -2 B (0%)
build/data-controls/index.js 1.29 kB +1 B
build/data/index.js 8.55 kB -2 B (0%)
build/date/index.js 5.38 kB +2 B (0%)
build/dom/index.js 4.48 kB +2 B (0%)
build/edit-navigation/index.js 11.6 kB +6 B (0%)
build/edit-post/index.js 304 kB -3 B (0%)
build/edit-site/index.js 17 kB +3 B (0%)
build/edit-widgets/index.js 11.8 kB +88 B (0%)
build/editor/index.js 45.3 kB -33 B (0%)
build/format-library/index.js 7.71 kB +4 B (0%)
build/is-shallow-equal/index.js 710 B -1 B
build/plugins/index.js 2.56 kB -1 B
build/shortcode/index.js 1.7 kB +1 B
build/url/index.js 4.06 kB +2 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 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/style-rtl.css 953 B 0 B
build/block-directory/style.css 952 B 0 B
build/block-editor/style-rtl.css 10.7 kB 0 B
build/block-editor/style.css 10.7 kB 0 B
build/block-library/theme-rtl.css 729 B 0 B
build/block-library/theme.css 730 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/style-rtl.css 15.7 kB 0 B
build/components/style.css 15.7 kB 0 B
build/compose/index.js 9.67 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 568 B 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/style-rtl.css 5.61 kB 0 B
build/edit-post/style.css 5.61 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/style-rtl.css 2.45 kB 0 B
build/edit-widgets/style.css 2.45 kB 0 B
build/editor/editor-styles-rtl.css 537 B 0 B
build/editor/editor-styles.css 539 B 0 B
build/editor/style-rtl.css 3.8 kB 0 B
build/editor/style.css 3.79 kB 0 B
build/element/index.js 4.65 kB 0 B
build/escape-html/index.js 733 B 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.56 kB 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/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/token-list/index.js 1.27 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

Copy link
Contributor

@talldan talldan left a comment

Choose a reason for hiding this comment

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

This is actually a surprisingly straightforward change, which is nice.

In terms of user experience, I think it may be confusing that there's an overlap between the block functionality. I wonder if we should limit 'Link' to just being external links, 'Page' to just being pages etc.

I personally don't expect to get other options when I add those blocks, especially the 'Link' block which gets suggestions for everything.

@noisysocks
Copy link
Member Author

In terms of user experience, I think it may be confusing that there's an overlap between the block functionality. I wonder if we should limit 'Link' to just being external links, 'Page' to just being pages etc.

I'll defer to @shaunandrews on this one 😀

@TimothyBJacobs
Copy link
Member

Why does this have a hardcoded list of post types and taxonomies?

Base automatically changed from add/formats-categories-suggestions to master August 25, 2020 03:41
@noisysocks noisysocks force-pushed the add/navigation-link-variations branch from b4a1023 to d42766c Compare August 25, 2020 04:14
Adds Post, Page, Category and Tag variations to the Navigation Link
block. Each variation sets the type attribute which in turn causes
LinkControl to filter its results using the /wp/v2/search API's type and
subtype params.
@noisysocks noisysocks force-pushed the add/navigation-link-variations branch from d42766c to 13a44f8 Compare August 25, 2020 04:17
@noisysocks
Copy link
Member Author

This has some test failures I need to fix up.

@noisysocks
Copy link
Member Author

Why does this have a hardcoded list of post types and taxonomies?

I started by implementing the most common Link variations: Page, Post, Category, Tag. You're probably right though that this should be dynamic so that we have a Link block variation for every (custom) post type and (custom) taxonomy.

@TimothyBJacobs
Copy link
Member

Why does this have a hardcoded list of post types and taxonomies?

I started by implementing the most common Link variations: Page, Post, Category, Tag. You're probably right though that this should be dynamic so that we have a Link block variation for every (custom) post type and (custom) taxonomy.

👍 Yeah I think that'd be a worthwhile change. Would also make sure any changes to the post type label are picked up.

@shaunandrews
Copy link
Contributor

Very cool to see this; I like having a block variation for each post type — though I worry a little about how this scales. Does a site with 20+ post type's defined then list 20+ link variations? If so, is that a problem?

I'd expect a Page Link block to only list pages in it's default suggests and search; and similar, a Post Link block would only show posts.

image

The more general catchall Link block should still be able to do it all, allowing the display of any post type along with supporting general URL links.

I tried creating some new icons that use the Navigation icon in an attempt to visually brand these blocks together, but I think using the existing, more generic icons might work just as good, it not better. The new icons can be found in the WP Design Library Figma:

image

@TimothyBJacobs
Copy link
Member

Very cool to see this; I like having a block variation for each post type — though I worry a little about how this scales. Does a site with 20+ post type's defined then list 20+ link variations? If so, is that a problem?

Yeah this can get annoying in Core currently. For example if you have a site with a lot of CPTs/Taxonomies and open the Menus screen you wind up with a bunch of meta boxes and the screen can get a bit unwieldy. You can hide them via Screen Options, and I think(?) only some are shown by default which can be really confusing for the user.

Making it a variation, though, feels like it might be a somewhat better experience? Since you are first searching & selecting the type and then that visual noise goes away.

@noisysocks noisysocks requested a review from ntwb as a code owner August 26, 2020 05:57
@noisysocks
Copy link
Member Author

This is ready for a re-review.

I rebased the PR to include #22600 and fixed all of the test failures.

I added the basic icons to each variation per @shaunandrews's suggestion above.

Screen Shot 2020-08-26 at 16 01 54

"Create page" will appear at the bottom of the Link and Page variations and "Create post" will appear at the bottom of the Post variation. No creation is possible in the Tag and Category variations. Linking to a user inputted URL is only possible in the Link variation.

Link Page Post Tag Category
Screen Shot 2020-08-26 at 16 03 43 Screen Shot 2020-08-26 at 16 03 59 Screen Shot 2020-08-26 at 16 03 30 Screen Shot 2020-08-26 at 16 04 18 Screen Shot 2020-08-26 at 16 04 31

I agree that a variation should be added for each custom post type and taxonomy, but I would like to do this later as it's a little bit tricky to implement.

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.

Code looks good and works like a charm! :shipit:

}
return createInterpolateElement(
sprintf( format, searchTerm ),
{ mark: <mark /> }
Copy link
Contributor

Choose a reason for hiding this comment

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

So nice 👍

@noisysocks
Copy link
Member Author

I agree that a variation should be added for each custom post type and taxonomy, but I would like to do this later as it's a little bit tricky to implement.

Issue here: #24814

@shaunandrews
Copy link
Contributor

At second glance, I don't think we need to post_type label in the search results if its only listing a single post_type.

@noisysocks
Copy link
Member Author

At second glance, I don't think we need to post_type label in the search results if its only listing a single post_type.

Issue here: #24839

@shaunandrews shaunandrews mentioned this pull request Aug 27, 2020
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Navigation Affects the Navigation Block Needs Design Feedback Needs general design feedback.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Navigation: Split Link block into Post, Page, Category, Link, etc.
5 participants