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: Improve global search UX #8249

Merged
merged 11 commits into from
Jun 25, 2020
Merged

UI: Improve global search UX #8249

merged 11 commits into from
Jun 25, 2020

Conversation

backspace
Copy link
Contributor

@backspace backspace commented Jun 23, 2020

This has a few improvements to the search:

  • new shortcut hint adapted from learn.hashicorp.com
  • fuzzy search with highlighting
    • this involves stuffing the Fuse “matches” into an Ember Data model which I don’t love but means the search results are always a collection of models vs a mix of models and Fuse wrappers
  • a debounce in the call to open the search to address the “focus blackhole”
  • adds a wrapper around the search so it can be centred instead of equally spaced
  • a fork of Ember Power Select to let me catch presses of escape to deactivate the search
    • using a fork is subpar, I know… I couldn’t figure out another way with the public API that exists 😞

This makes things a bit off in all the browser’s I’ve checked
with so it needs some tweaking.
Maybe this fixes the click-doesn’t-open-search problem?
@backspace backspace added this to the 0.12.0-beta2 milestone Jun 23, 2020
@backspace backspace self-assigned this Jun 23, 2020
@github-actions
Copy link

github-actions bot commented Jun 23, 2020

Ember Asset Size action

As of fd39ead

Files that got Bigger 🚨:

File raw gzip
nomad-ui.js +5.07 kB +541 B
vendor.js +8 B +47 B
nomad-ui.css +977 B +120 B

Files that stayed the same size 🤷‍:

File raw gzip
auto-import-fastboot.js 0 B 0 B
vendor.css 0 B 0 B

@github-actions
Copy link

github-actions bot commented Jun 23, 2020

Ember Test Audit comparison

master fd39ead change
passes 1425 1429 +4
failures 0 0 0
flaky 0 0 0
duration 5m 21s 141ms 5m 09s 604ms -11s 537ms

This uses a hacked version of Ember Power Select that adds
another option to its public API: setIsActive. This allows
me to check whether the escape key is what led to a close
event and change the control to be inactive if so.
@backspace backspace force-pushed the b-ui/search-fixes branch from 51b9e96 to a940fe1 Compare June 24, 2020 14:04
Maybe the problem was a double-open?
@backspace backspace force-pushed the b-ui/search-fixes branch from 9eed9fa to f892486 Compare June 24, 2020 18:01
This causes it to run into the navigation elements so maybe
I’ll hide it at a wider breakpoint? 🧐
I don’t love the look of this… adapted from learn.hashicorp.com
but the colour scheme seems less favourable 😞
@backspace backspace marked this pull request as ready for review June 24, 2020 20:30
Copy link
Contributor

@DingoEatingFuzz DingoEatingFuzz 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 a nice collection of improvements.

The fork is of course unfortunate. Hopefully that can be handled upstream at some point.

The result highlighting is also a little funky. It doesn't always highlight the text I would expect. But short of doing what github's file fuzzy search does (break the search term into chunks and highlight multiple substrings) I think this is the best we can get.

⚠️ subjective aesthetic nitpicking follows ⚠️

The border around the search input in the dormant state is better now, but I think it might still be too much. The contrast draws a lot of attention to it when I think it would be okay for it to fall into the background a bit more. The original design for this had it borderless when idle.

image

As for the shortcut icon, I think it may be suffering the same border problem where it's drawing a lot of focus by the sharp lines. Maybe a dark green "/" on a light green, rounded background?

These are just things to think about between now and RC, definitely not blocking this on that 🙂

@DingoEatingFuzz
Copy link
Contributor

Mock up of how I was thinking the shortcut indicator could look:

image

@backspace
Copy link
Contributor Author

Thanks for the styling suggestions, I meant to refer back to the original design but failed to in the press for time. I think it looks much better now!

image

Re fuzzy search highlighting expectations, it should be possible to have multiple highlights instead of only using the first. Can you give an example of not highlighting what you would expect? I think this would be a minimal intervention before the RC 🤞

@backspace backspace merged commit fe445a0 into master Jun 25, 2020
@backspace backspace deleted the b-ui/search-fixes branch June 25, 2020 13:51
@DingoEatingFuzz
Copy link
Contributor

Here's one:
image

I think this happens whenever you start typing the second word but the first letter is in the prefix.

I think the way github's match highlighter would would this is smtp-sensor-1

@backspace
Copy link
Contributor Author

excellent, thanks, I can definitely refine the match component 😀

backspace added a commit that referenced this pull request Jun 30, 2020
The CSS I added in #8249 to make the search be properly
centred also made the logo unclickable as it was hidden
behind the centred element! This makes the logo stay
above the search container.
backspace added a commit that referenced this pull request Jun 30, 2020
The CSS I added in #8249 to make the search be properly
centred also made the logo unclickable as it was hidden
behind the centred element! This makes the logo stay
above the search container.
@github-actions
Copy link

github-actions bot commented Jan 2, 2023

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants