search

biocore / emperor

Emperor a tool for the analysis and visualization of large microbial ecology datasets
http://biocore.github.io/emperor/
Other
52 stars 50 forks source link

Add a search bar to the controllers #735

Closed ElDeveloper closed 4 years ago

ElDeveloper commented 5 years ago

This PR adds a search bar to filter down values in any given category. This is useful for large datasets. Here's an animated gif showcasing how the search bar works in the color, visibility and scale controllers:

search-bar

The bulk of the changes are in view-controller.js and a lot of other minor changes happened due to updated code style guidelines.

thermokarst commented 5 years ago

I would be happy to review, if you're looking for reviewers!

ElDeveloper commented 5 years ago

@thermokarst, yes that would be wonderful!

antgonza commented 5 years ago

Thank @ElDeveloper, looking great!

Now, one minor thing: when there is no category selected the text bar is shown with the text "Search for a value" but is not clickable; what about hiding it until is usable or at least changing the text?

Also, how will these changes interact with the other parallel-plots PR? My concern were the menus and if they stayed in the previous state. Note that I tried to test but ran into conflicts CONFLICT (content): Merge conflict in tests/javascript_tests/test_color_view_controller.js) so I guess the easiest will be to merge one, update the other, and retest ...

ElDeveloper commented 5 years ago

Now, one minor thing: when there is no category selected the text bar is shown with the text "Search for a value" but is not clickable; what about hiding it until is usable or at least changing the text?

Great suggestion, I'll work on this.

Also, how will these changes interact with the other parallel-plots PR? My concern were the menus and if they stayed in the previous state. Note that I tried to test but ran into conflicts CONFLICT (content): Merge conflict in tests/javascript_tests/test_color_view_controller.js) so I guess the easiest will be to merge one, update the other, and retest ...

I think the search terms might need to be included as part of the UIState object (from the parallel plots). So I think you are right, we need to merge one or the other.

ElDeveloper commented 4 years ago

Thanks @thermokarst! @antgonza this should be ready to merge whenever you get a chance.

antgonza commented 4 years ago

Thank you @ElDeveloper and @thermokarst !

ElDeveloper commented 4 years ago

Thanks @antgonza!

On (Oct-22-19| 8:20), Antonio Gonzalez wrote:

Merged #735 into new-api.

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/biocore/emperor/pull/735#event-2733739774