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 improvements with global search tool #1039

Merged
merged 7 commits into from May 23, 2016
Merged

UI improvements with global search tool #1039

merged 7 commits into from May 23, 2016

Conversation

@wolfd
Copy link
Contributor

@wolfd wolfd commented May 13, 2016

Makes a number of improvements to the global search tool:

  • The search tool is brought up immediately instead of waiting for the backbone collection to be loaded
  • Pseudo-tab completion is possible (it picks and navigates to the currently selected suggestion, or defaults to the first value in the list if nothing is highlighted)
  • Hitting enter does the same thing as Tab, immediately navigating to the result

I want to add one more thing into this PR before I'm satisfied with it, and that is highlighting the portions of each result that are being matched by the fuzzy search. This was present with twitter-typeahead, but requires some additional work with react-typeahead.

globalSearchClasses = [
'global-search'
'global-search-active' if @props.visible
].join(' ')
Copy link
Member

@tpetr tpetr May 15, 2016

Choose a reason for hiding this comment

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

Loading

@wolfd
Copy link
Contributor Author

@wolfd wolfd commented May 18, 2016

Here's a screenshot of the new UI, not much has changed.
The highlighted text is bolder, and all of the options are a tags now.

screen shot 2016-05-18 at 1 06 21 pm

In keybinding world, you should be able to just hit enter without hitting the down arrow first if the first option is what you want.

Also, if you hit escape before finishing a search, it doesn't reset the search box when you bring it back up. I'm not sure if this is an improvement or not, it depends on the particular workflow of whoever is using it.

In a later PR, we can make tab move down the list, but that isn't supported by react-typeahead yet, so it will take another PR there.

Loading


render: =>
if @props.visible
@focus()
Copy link
Member

@tpetr tpetr May 18, 2016

Choose a reason for hiding this comment

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

the more idiomatic way to do this would be to use componentDidUpdate(), and only call @focus() if the visible prop transitioned from false to true (as a general rule of thumb, you shouldn't be manipulating the DOM or triggering events like that in render())

Loading

@wolfd wolfd added the hs_qa label May 18, 2016
@ssalinas ssalinas added this to the 0.8.0 milestone May 19, 2016
@ssalinas ssalinas added this to the 0.8.0 milestone May 19, 2016
@tpetr tpetr merged commit 5c47afe into master May 23, 2016
2 checks passed
Loading
@tpetr tpetr removed hs_qa labels May 23, 2016
@wolfd wolfd deleted the improve_global_search branch May 23, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants