Skip to content

search: Refactor search context global state 1/n

Warren Gifford requested to merge fkling/search-context-refactor into main

Created by: fkling

This first PR refactors the props used by the search context CTA only. There are couple of things going on here and they probably need some discussion:

  • The SearchContextCtaPrompt component is not directly rendered by the SearchContextDropdown anymore. The CTA is only used in the web app, but the search context dropdown is also used in our vscode extension. Instead of controlling the visibility of the CTA via the props that the CTA uses (which requires knowledge about implementation details), SearchBox now accepts the CTA as a prop itself. This makes the component hierarchy flatter and allows allows the vscode extension to simply pass nothing.

  • While the global state flags (hasUserAddedRepositories, etc) are not necessarily search context specific, they have been added and are only used by search context at the moment. Additionally, logic had to be added to update those values. I'm worried that by storing state in a "general" place (like the app root, or a store just for those flags), other features will eventually use flags without necessarily understanding their context/purpose. Instead I'm proposing to adopt the concepts from the Flux architecture: Components can dispatch actions/events which are handled by an arbitrary number of stores. This leads to less coupled code by separating state change intentions (events/actions) from their effect (search context cta state change). Specifically the dispatch function dispatches the event/action and the search context cta store registers itself with the dispatcher.

  • The SearchContextCtaContainer provides the glue between the shared component and the state implementation of the web app (this is not the best example since the web app is the only platform to show the CTA at this time).

Note: The structure of the code is a first draft. I don't want to imply that this is how we should structure events/actions (should probably standardize on "actions"). Everything is up for debate.

Test plan

App preview:

Check out the client app preview documentation to learn more.

Merge request reports