Search ability in mobile, display no results found, no right chevron in mobile #248
Conversation
|
@djsauble would love your feedback on these since I mostly just guessed on what the design should look like! |
src/components/NavigationItems.js
Outdated
| @@ -123,7 +131,7 @@ const NavItem = ({ page, depthLevel, searchTerm, filteredPageNames }) => { | |||
| onKeyPress={() => setIsExpanded(!isExpanded)} | |||
| tabIndex={0} | |||
| > | |||
| {depthLevel > 0 && ( | |||
| {!mobile && depthLevel > 0 && ( | |||
There was a problem hiding this comment.
Rather than adding a new prop, passing it around, and doing these conditional checks, can we just target the chevron with CSS? We're already showing / hiding the mobile menu with a media query.
Co-authored-by: Zack Stickles <[email protected]>
| @@ -35,6 +35,10 @@ const Navigation = ({ className, searchTerm }) => { | |||
| ? filterPageNames(pages, searchTermSanitized) | |||
| : undefined; | |||
|
|
|||
| if (filteredPageNames?.length === 0) { | |||
There was a problem hiding this comment.
You may have a potential bug here. Is there ever a case where filteredPageNames is undefined or null? If so, comparing to 0 will result in false, thereby not showing the "No results found" message. If you can make sure filteredPageNames is always an empty array, then that should ensure this message is always shown at the appropriate time and would allow you to remove the ?..
There was a problem hiding this comment.
filteredPageNames will always return an array when you are searching, otherwise it will be undefined. When it is undefined, it will not have a property of length and then filteredPageNames?.length === 0 will evaluate to false! Otherwise, if it is being searched, then there will either be an array with values or an empty array. When it is an empty array, it will return "No results found." So I don't believe there is a bug possible.
| @@ -35,6 +35,10 @@ const Navigation = ({ className, searchTerm }) => { | |||
| ? filterPageNames(pages, searchTermSanitized) | |||
| : undefined; | |||
|
|
|||
| if (filteredPageNames?.length === 0) { | |||
| return <div className={styles.emptyResults}>No results found</div>; | |||
There was a problem hiding this comment.
Suggest italicizing this just to emphasize that it's not a sidebar nav item
|
🎉 This PR is included in version 1.0.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Description
Addresses various bug fixes for search in sidenav/mobile.
Reviewer Notes
Reduce screen size to see mobile view, will see search bar there. Can type in random keys to get a no match for "no results found." Select a random page and reduce screen size to see the bolded page and the right chevron removed.
Related Issue(s) / Ticket(s)
Screenshot(s)
Search functionality in mobile menu
"No results found" messaging
Removed right chevron