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

UI: Fixes for Sidebar and Search #13027

Merged
merged 12 commits into from
Nov 6, 2020
Merged

UI: Fixes for Sidebar and Search #13027

merged 12 commits into from
Nov 6, 2020

Conversation

ghengeveld
Copy link
Member

Issue: #13000

What I did

  1. Update empty state for search: show a “no results” message. Currently we show no results and no messages which makes the UI appear broken.

I added a "No components found" message:

Screenshot 2020-11-05 at 21 12 17

  1. No recently opened: When there are no recently opened items continue showing the treeview. Currently we show nothing so it looks like the treeview completely disappeared. You can reproduce this state by clearing recently opened.

Instead of showing the treeview, I made sure that the history will always show at least one item. After all, you will always have a component selected, so technically that one should always be at the top of the list.

  1. When you click on the expand all button, keyboard navigation no longer behaves as I'd expect. Every time I press enter the focus returns to the expand all button no matter what I focus on using my keyboard. Video here

Fixed this by removing focus from the expand-all button when using the arrow keys to navigate the tree.

  1. Keyboard navigation skips items when a folder is expanded and collapsed. Video here. When expanding and collapsing using the right and left arrow keys I expect the focus to start at the very beginning of the list not in the middle.

This was a bug and is fixed now.

  1. Keyboard navigation focus doesn't transfer to the "folder" items. Video here

Fixed. Now when you click a node to expand/collapse it, it will get highlighted as well.

How to test

See the above descriptions and videos for manual QA.

Copy link
Member

@domyen domyen left a comment

Choose a reason for hiding this comment

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

Nice, looks good. Suggested minor UI tweaks in the comments. Request below:

Recently opened back button
I realize that there's no obvious way to get back to the components list from the "Recently opened" view. Video description

To make it dead obvious, add a back button "« Back to components" above clear history. Perhaps it'd also make sense to allow the [esc] key to go back from this state as well.

image

lib/ui/src/components/sidebar/SearchResults.tsx Outdated Show resolved Hide resolved
@ghengeveld
Copy link
Member Author

Done 👍

@shilman shilman added this to the 6.1 search milestone Nov 5, 2020
Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

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

Build failing due to type errors:

@storybook/ui: src/components/sidebar/Search.stories.tsx(35,13): error TS2741: Property 'closeMenu' is missing in type '{ results: any; query: string; getMenuProps: (props?: any) => { key: any; }; getItemProps: (props?: any) => { key: any; }; highlightedIndex: number; }' but required in type '{ query: string; results: DownshiftItem[]; closeMenu: (cb?: () => void) => void; getMenuProps: (options?: GetMenuPropsOptions, otherOptions?: GetPropsCommonOptions) => any; getItemProps: (options: GetItemPropsOptions<...>) => any; highlightedIndex: number; }'.
@storybook/ui: src/components/sidebar/Search.stories.tsx(42,8): error TS2741: Property 'closeMenu' is missing in type '{ query: string; results: DownshiftItem[]; getMenuProps: (options?: GetMenuPropsOptions, otherOptions?: GetPropsCommonOptions) => any; getItemProps: (options: GetItemPropsOptions<...>) => any; highlightedIndex: number; }' but required in type '{ query: string; results: DownshiftItem[]; closeMenu: (cb?: () => void) => void; getMenuProps: (options?: GetMenuPropsOptions, otherOptions?: GetPropsCommonOptions) => any; getItemProps: (options: GetItemPropsOptions<...>) => any; highlightedIndex: number; }'.
@storybook/ui: src/components/sidebar/SearchResults.stories.tsx(80,33): error TS2741: Property 'closeMenu' is missing in type '{ query: string; results: { item: SearchItem; matches: { value: string; indices: number[][]; }[]; score: number; }[]; getMenuProps: (props?: any) => { key: any; }; getItemProps: (props?: any) => { key: any; }; highlightedIndex: number; }' but required in type '{ query: string; results: DownshiftItem[]; closeMenu: (cb?: () => void) => void; getMenuProps: (options?: GetMenuPropsOptions, otherOptions?: GetPropsCommonOptions) => any; getItemProps: (options: GetItemPropsOptions<...>) => any; highlightedIndex: number; }'.
@storybook/ui: src/components/sidebar/SearchResults.stories.tsx(82,33): error TS2741: Property 'closeMenu' is missing in type '{ results: any; query: string; getMenuProps: (props?: any) => { key: any; }; getItemProps: (props?: any) => { key: any; }; highlightedIndex: number; }' but required in type '{ query: string; results: DownshiftItem[]; closeMenu: (cb?: () => void) => void; getMenuProps: (options?: GetMenuPropsOptions, otherOptions?: GetPropsCommonOptions) => any; getItemProps: (options: GetItemPropsOptions<...>) => any; highlightedIndex: number; }'.
@storybook/ui: src/components/sidebar/SearchResults.stories.tsx(84,34): error TS2741: Property 'closeMenu' is missing in type '{ query: string; results: { item: SearchItem; matches: any[]; score: number; }[]; getMenuProps: (props?: any) => { key: any; }; getItemProps: (props?: any) => { key: any; }; highlightedIndex: number; }' but required in type '{ query: string; results: DownshiftItem[]; closeMenu: (cb?: () => void) => void; getMenuProps: (options?: GetMenuPropsOptions, otherOptions?: GetPropsCommonOptions) => any; getItemProps: (options: GetItemPropsOptions<...>) => any; highlightedIndex: number; }'.

Can you please fix?

@ghengeveld
Copy link
Member Author

Tests are failing for some reason not related to this PR.

@shilman shilman merged commit 9d5071b into next Nov 6, 2020
@shilman shilman deleted the 13000-sidebar-rfcs branch November 6, 2020 13:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants