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 Component: What to Search and How #25509

Closed
Copons opened this issue Sep 21, 2020 · 2 comments
Closed

Navigation Component: What to Search and How #25509

Copons opened this issue Sep 21, 2020 · 2 comments
Labels
[Feature] Navigation Component A navigational waterfall component for hierarchy of items.

Comments

@Copons
Copy link
Contributor

Copons commented Sep 21, 2020

This issue depends on #25252, and was spurred by a discussion happened in #25315 (#25315 (comment) and following comments).

The plan (WIP in #25315) is to make Navigation searchable: typing into a search field would filter the items, keeping only those items that contain the search query.

Normally, NavigationItem is used with a title prop (rendered as item label), so it's just obvious to search on that.

But NavigationItem can also render a custom component, which makes the title not mandatory (and not rendered anyway).
We could search into the custom component's text content, although it could be expensive performance-wise on large navigation lists: for example, we might need to store its ref, or recursively traverse its DOM.

Possibilities to discuss:

  • Require title for custom component items, even if it's not rendered.

    • Pros: easy to develop and to consume.
    • Cons: counterintuitive to use; inconsistent for the end user if title and content don't match.
  • Add a new keywords prop, to be used simlarly to Blocks keywords. We could search on those if no title is provided, or even in addition to it.

    • Pros: precise search results.
    • Cons: yet another thing the consumer will need to remember; inconsistent for the end user if keywords and content don't match.
  • Store the custom component ref and search into Node.textContent().

    • Pros: easy to develop; consumers wouldn't need to do anything.
    • Cons: possibly bad performances; wouldn't work for custom components without text (e.g. images, icons).
  • Traverse recursively through the custom component children.

    • Pros: consumer wouldn't need to do anything; we might customize the traversal to pick up non-text content (e.g. alt attributes).
    • Cons: possibly bad performances.
@Copons Copons added the [Feature] Navigation Component A navigational waterfall component for hierarchy of items. label Sep 21, 2020
@mattwiebe
Copy link
Contributor

Since both search and custom components are unknown quantities at this point, it's hard to say what the best path forward is until we have some a bunch of consumers of the component. Therefore, I believe that our main priorities should be simplicity of use for consumers and the ability to grow in complexity as needed.

With that in mind, some thoughts on each approach:

Require title for custom component items, even if it's not rendered

This seems like the most straightforward approach. We might even consider making it optional: "supply a title attribute to include your custom component in search results" - but I guess strictness will lead to an overall better UX

Add a new keywords prop, to be used similarly to Blocks keywords.

If we didn't already have the title prop, this would be useful, but I would only do so if we were using it everywhere.

Store the custom component ref and search into Node.textContent()

I doubt that the performance hit would be that large, but then again, we haven't scaled Navigation up to hundreds or thousands of entries. And that's the area where search's utility comes to the fore. We'd need to do some perf testing to see if this is viable, but I just don't see the effort being worthwhile when the title prop is easy and straightforward.

Traverse recursively through the custom component children

Same as above. Don't see the real benefits.


In conclusion, we are already requiring a title prop for regular NavigationItems, we can just carry that forward and leverage for search. Consistency > cleverness.

@Copons
Copy link
Contributor Author

Copons commented Sep 25, 2020

Cool, thanks for looking into this!
I've updated #25315 to mention that title will be used by the search.

I'll close this issue.

@Copons Copons closed this as completed Sep 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Navigation Component A navigational waterfall component for hierarchy of items.
Projects
None yet
Development

No branches or pull requests

2 participants