Navigation Component: Handle the no search results state #27160
Conversation
Copons
requested review from
mattwiebe,
vindl,
david-szabo97,
jeyip,
noahtallen and
Addison-Stavlo
Copons
added
[Feature] Navigation Component
|
Size Change: +56 B (0%) Total Size: 1.2 MB
ℹ️ View Unchanged
|
|
Thanks for this Jacopo! The story works well but in our use-case, it announces "0 results found" right after typing in something. We are debouncing the search which causes it to have 0 results for a brief moment.
We could remove the denounce for now for a quick win but I'd rather not. Any idea how to make this debounce-friendly? I kinda feel like we can't without passing in a prop. Or we can build the denounce into the component. Passing it as a prop like |
|
Quick tips to those trying to reproduce the issue posted by David:
I think it's worth noting that if we lower the debounce enough, I'm not able to replicate the "0 results found" screen reader problem. I am, however, able to replicate another issue that David pointed out elsewhere -- we still see the "no results found" item popping up between queries. |
I've spent some time mulling this over and I'm inclined to agree. I'll experiment with passing a debounce prop through and if we're not fans, we can revert whatever I commit. |
jeyip
force-pushed
the
fix/navigation-no-results-found
branch
4 times, most recently
from
aa8009f to
72c63ae
Compare
|
The code I pushed fixes the flashing "No results found" message in between debounces while still implementing the fix originally intended by this PR. I do think, however, that the code is definitely up for discussion. Would love to hear feedback and thoughts @david-szabo97 and @Copons |
|
Thanks for looking into this @jeyip! ✨ The search PR (#26665) currently just filters the templates already available in state, it doesn't fire new requests on type, so there should be no real need of a debounce anyway for now. The day we see that front-loading templates is unmanageable, we'll need a refactor of many things (e.g. dynamically loaded nav sidebar, pagination, debounced search, etc.). I'm going to revert your commits in order to, hopefully, get the search feature merged ASAP. |
Copons
force-pushed
the
fix/navigation-no-results-found
branch
from
3931b87 to
62b5022
Compare
Description
A missing feature for the built-in search of the Navigation component is handling the "no results" state.
The component is smart enough to count the results by itself, and announce them to screen readers.
But when we implemented it in a real case scenario (#26665), we had to create a "no results" message from scratch.
We tried rendering it as a simple, text-only,
NavigationItem, which worked fine visually, but unfortunately was counted as a search result, and announced as "1 result found" to screen readers.This PR adds a "No results found." message, visually identical to any other
NavigationItem, but not counted towards the results.It's only rendered if the search is active and "in progress" (there are characters in the search field), and there are no results.
How has this been tested?
npm run storybook:devand search for the Navigation > Search story.Types of changes
New feature (non-breaking change which adds functionality)
Checklist: