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

Stateful search bar default behaviors #56160

Merged

Conversation

lizozom
Copy link
Contributor

@lizozom lizozom commented Jan 28, 2020

Summary

The goal of the stateful version of SearchBar \ TopNavMenu is to be an easy way to consume the services offered by plugins.data.query where developers should be able to provide minimal configuration and get a fully working SearchBar.

I used the discover application as a sample app integration with the stateful component. (These changes were tested on visualize \ dashboard, but to keep this PR as small as possible, I left them out).

The React equivalent of the API used by discover is:

    <data.ui.SearchBar 
         screenTitle="My App"
         showSearchBar={true}
         showDatePicker={enableTimeRangeSelector}
         indexPatterns={[indexPattern]} //Do we want to support multiple index patterns?

         // optional initial state - can be also set directly via services 
         filters={filters}
         query={query}
         savedQueryId={savedQueryId}

         // optional callbacks, to be replaced with observables.
         onQuerySubmit={updateQuery} 
         onSavedQueryIdChange={updateSavedQueryId}

         // For scoping purposes, this is opt in ATM.
         useDefaultBehaviors={true}
     />

It is important to note that the SearchBar API is still undergoing changes so there might be still some inconsistencies to scope this PR

Current state (on master)

These features have default behaviors on master

  • Managing filters ✔️
  • Being able to switch query languages ✔️
  • Autocompletion of KQL queries and of purposed fields in the new filters dialog ✔️
  • Configuring auto refresh ✔️

These features don't have a default implementation

  • Managing timefilter ✖️
  • Managing query strings ✖️
  • Saved Queries ✖️

Hence when configuring a new SearchBar (or TopNav) a developer had to provide an implementation for behaviors that don't have default behaviors.

Moreover, most application haven't fully utilized this capability because it was partial and undocumented.

Default Behaviors

This PR adds all missing default behaviors and utilizes them to clean up discover. In order not to refactor all places using SearchBar \ TopNavMenu, I made it opt-in using the flag useDefaultBehavior.

Filters

  • A plugin doesn't have to pass in filters at all, as their initial state is synced into data.query.filterManager via state.
  • When a user changes any filters via the FilterBar, their state is automatically stored into data.query.filterManager.
  • Since data.query.filterManager posts updates via an observable, you cannot pass in an additional callback into the stateful SearchBar.
  • For more information, lookup defaultFiltersUpdated

Timefilter \ Autorefresh settings

  • A plugin doesn't have to pass in timefilter or autorefresh settings at all, as their initial state is synced into data.query.timefilter via state.
  • When a user changes the time range \ autorefresh settings via the Timepicker, their state is automatically stored into data.query.timefilter.
  • timefilter posts updates via an observable, which is the preferred way to track changes. This is the only way to get notified of autorefresh changes, however, passing in a custom onQuerySubmit callback is still supported.
  • For more information, lookup defaultOnQuerySubmit and defaultOnRefreshChange

Query String settings

  • Since we still don't have a proper query string manager, query initial state must be passed into SearchBar.
  • When a user changes the query string via the Timepicker, its state is automatically stored into the stateful SearchBar (will be moved to query string manager when available).
  • At the moment, legacy query migration is still performed inside the application, but this can be easily moved into the stateful component.
  • Query String changes are still notified via a callback onQuerySubmit and should be tracked by application state, since there's no API to retrieve them from data plugin.
  • For more information, lookup defaultOnQuerySubmit.

Saved Queries

  • Saved queries workflow is fully contained within the stateful component and requires no input or configuration.
  • You may provide callbacks if you are interested in notifications from various stages.
  • For more information, lookup defaultOnClearSavedQuery, defaultOnSavedQueryUpdated, defaultOnQuerySaved.

Next steps

  • Implement a full query string service that will allow changing the query string programmatically.
  • Return an observable from that service and stop using the onQuerySubmit as the only mean of getting updates on query changes.
  • Consider a single observable that notifies of any changes to the inner state of SearchBar

Dev Docs

The goal of the stateful version of SearchBar \ TopNavMenu is to be an easy way to consume the services offered by plugins.data.query where developers should be able to provide minimal configuration and get a fully working SearchBar.

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

For maintainers

@lizozom lizozom self-assigned this Jan 28, 2020
@lizozom lizozom added release_note:plugin_api_changes Contains a Plugin API changes section for the breaking plugin API changes section. Team:AppArch v7.7.0 v8.0.0 labels Jan 28, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-arch (Team:AppArch)

@lizozom lizozom changed the title Improve stateful search bar 2 Stateful search bar default behaviors Jan 29, 2020
@lizozom lizozom marked this pull request as ready for review January 29, 2020 16:04
@lizozom lizozom requested a review from a team January 29, 2020 16:04
@lizozom lizozom requested a review from a team as a code owner January 29, 2020 16:04
@lizozom lizozom requested a review from majagrubic January 29, 2020 16:10
@kertal
Copy link
Member

kertal commented Feb 4, 2020

thx for fixing the issues and the tests 👍 I hopefully noticed one last thing while testing, Context is now displaying the whole query bar

Before:
Bildschirmfoto 2020-02-04 um 13 01 25
Now:
Bildschirmfoto 2020-02-04 um 12 54 11

@lizozom
Copy link
Contributor Author

lizozom commented Feb 4, 2020

@kertal committed a fix for this literally 30 seconds ago

Copy link
Contributor

@nreese nreese left a comment

Choose a reason for hiding this comment

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

lgtm

@kertal
Copy link
Member

kertal commented Feb 4, 2020

@lizozom this fixed the context display problem, guess the hopefully last problem here is the one you're currently working on: can't add filters in context view

Liza K added 3 commits February 4, 2020 19:59
Improve useSavedQuery hook in terface
…izozom/kibana into newplatform/data/stateful-search-bar-2
@lizozom
Copy link
Contributor Author

lizozom commented Feb 4, 2020

@kertal I think I managed to stabilize this, but lets wait until CI is green :)

Copy link
Member

@kertal kertal left a comment

Choose a reason for hiding this comment

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

Code LGTM, context now works as expected! Thx a lot for cleaning up ancient discover/context code!

@lizozom
Copy link
Contributor Author

lizozom commented Feb 5, 2020

@elasticmachine merge upstream

@lizozom lizozom merged commit 1fa01a7 into elastic:master Feb 6, 2020
lizozom pushed a commit to lizozom/kibana that referenced this pull request Feb 6, 2020
* Clean up discover

* Clean up visualize

* Clean up dashboard

* use-default-behaviors

* ts

* Discover interval changing

* timerange for interval definition in editor

* ts

* Revert most of changes to dashboard app because of changes in elastic#55443

* Fix spaces

* Revert editor to scope PR!

* typo

* keep savedQuery state in create search bar

* update saved query to save it with the state

* revert all dashboard changes

* saved queries

* @kertal code review

* fix applying filters from histogram

* @Dosant code review

* Merge changes from elastic#56643 to handle saved query errors

Fix bug where saved query clean does not reset query

* change string path

* if

* Extract useFilterManager and useTimefilter

* Split useSavedQuery and restore capability of changing saved query in URL

* Added some tests

* context view

* Remove useMemo

* spaces

* Support filter intial values
Improve useSavedQuery hook in terface

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
lizozom pushed a commit that referenced this pull request Feb 6, 2020
* Clean up discover

* Clean up visualize

* Clean up dashboard

* use-default-behaviors

* ts

* Discover interval changing

* timerange for interval definition in editor

* ts

* Revert most of changes to dashboard app because of changes in #55443

* Fix spaces

* Revert editor to scope PR!

* typo

* keep savedQuery state in create search bar

* update saved query to save it with the state

* revert all dashboard changes

* saved queries

* @kertal code review

* fix applying filters from histogram

* @Dosant code review

* Merge changes from #56643 to handle saved query errors

Fix bug where saved query clean does not reset query

* change string path

* if

* Extract useFilterManager and useTimefilter

* Split useSavedQuery and restore capability of changing saved query in URL

* Added some tests

* context view

* Remove useMemo

* spaces

* Support filter intial values
Improve useSavedQuery hook in terface

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
@kibanamachine
Copy link
Contributor

💔 Build Failed

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:New Platform release_note:plugin_api_changes Contains a Plugin API changes section for the breaking plugin API changes section. v7.7.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants