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

[Controls] Redux Toolkit and Embeddable Redux Wrapper #114371

Merged
merged 46 commits into from
Oct 14, 2021

Conversation

ThomThomson
Copy link
Contributor

@ThomThomson ThomThomson commented Oct 7, 2021

Summary

Closes #101194

Redux Embeddable Wrapper

system which can be used to give any embeddable the ability to manage its state via Redux without the hassle of setting up a new store, building a slice, or dealing with syncing redux state and embeddable state. Embeddables that wish to use redux can now do so with the following steps:

  1. Write reducers for the embeddable's input type
  2. When rendering the embeddable, wrap its top level component in the ReduxEmbeddableWrapper, providing the input type and the reducers.
  3. In any component under the Embeddable, use the useReduxEmbeddableContext hook to access the dispatch, typed actions, and selector.
  4. to get access to the current state use the selector, to update the state, use the dispatch and an action.

Redux state in Control Group

The control group has been updated to use Redux with the Redux embeddable wrapper. This greatly simplifies state management in the control group embeddable. It's behaviour should be the same, with a few small exceptions:

  • Set all widths: In the Group management screen, the button group which used to set all control widths now sets the default width, a new button to the right of it sets all control widths to the default.

Screen Shot 2021-10-07 at 5 48 24 PM

  • Confirm Modals: The confirm modals now only appear when exiting a control edit or create flyout without saving your progress AND when you have unsaved changes.

Screen Shot 2021-10-07 at 5 50 19 PM

Preconfigured story

The storybook now has story with a control group pre-configured. This makes development faster, and makes it easier to check out the progress at a glance!

Screen Shot 2021-10-07 at 5 51 14 PM

ThomThomson and others added 30 commits August 12, 2021 15:00
… Delete all functionality with confirm. Reset width after canceling edit
[Controls] dnd clone design improvements
Co-authored-by: Andrea Del Rio <delrio.andre@gmail.com>
@ThomThomson ThomThomson changed the base branch from master to 7.15 October 12, 2021 17:58
@ThomThomson ThomThomson changed the base branch from 7.15 to master October 12, 2021 17:58
@ThomThomson
Copy link
Contributor Author

@elasticmachine merge upstream

@ThomThomson ThomThomson marked this pull request as ready for review October 13, 2021 14:13
@ThomThomson ThomThomson requested a review from a team as a code owner October 13, 2021 14:13
@ThomThomson ThomThomson added Feature:Dashboard Dashboard related features Feature:Input Control Input controls visualization impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. loe:large Large Level of Effort release_note:skip Skip the PR/issue when compiling release notes Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas v8.0.0 labels Oct 13, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-presentation (Team:Presentation)

> => ({
diffInput: (a, b) => {
const differences: Partial<InputType> = {};
const allKeys = [...Object.keys(a), Object.keys(b)] as Array<keyof InputType>;
Copy link
Contributor

Choose a reason for hiding this comment

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

is there a missing spread operator for the Object.keys(b) here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wow, what a good catch... that could've made for some annoying and hard to diagnose errors down the line! I've fixed that.

Copy link
Contributor

@poffdeluxe poffdeluxe left a comment

Choose a reason for hiding this comment

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

There's a lot going on here but overall looks good to me. I had one question regarding that spread operator but other than that this seems like it's going to make embeddable state management much easier

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
presentationUtil 121 124 +3

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
presentationUtil 150 151 +1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
presentationUtil 43.1KB 43.1KB -6.0B

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id before after diff
presentationUtil 5 6 +1

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
presentationUtil 43.0KB 43.4KB +386.0B
Unknown metric groups

API count

id before after diff
presentationUtil 177 178 +1

History

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

@ThomThomson ThomThomson merged commit f8cbbbb into elastic:master Oct 14, 2021
jloleysens added a commit to jloleysens/kibana that referenced this pull request Oct 14, 2021
…mple/introduce-baseline-tests

* 'master' of github.com:elastic/kibana: (55 commits)
  [Fleet] Improve Functionality around Managed Package Policies (elastic#114526)
  cleanup (elastic#114902)
  remove stray semicolon (elastic#114969)
  [Security Solution] Edit host isolation exception IP UI (elastic#114279)
  [ML] APM Correlations: Round duration values to be used in range aggregations. (elastic#114833)
  [Index Management] Added `data-test-subj` values to the index context menu buttons (elastic#114900)
  [Stack monitoring] Fix logstash functional tests for react (elastic#114819)
  Implement hybrid approach to writing rule execution event logs (elastic#114852)
  [Detection Rules] Add 7.16 rules (elastic#114939)
  Fixing exceptions export format (elastic#114920)
  Clean up inaccurate comments (elastic#114935)
  chore(NA): fixes a typo on persist_bazel_cache.sh comment (elastic#114943)
  [ci] Fixes Bazel cache writes (elastic#114915)
  fix package.json: (elastic#114936)
  [Controls] Redux Toolkit and Embeddable Redux Wrapper (elastic#114371)
  [APM] Fixes incorrect index config names (elastic#114901) (elastic#114904)
  [Workplace Search] Fix button order and remove extra source name label (elastic#114899)
  [Actions] Fixed actions telemetry for multiple namespaces usage (elastic#114748)
  docs: fix config names (elastic#114903)
  Update kibana to EMS 7.16 (elastic#114865)
  ...
@ThomThomson ThomThomson added the backport:skip This commit does not require backporting label Oct 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting Feature:Dashboard Dashboard related features Feature:Input Control Input controls visualization impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. loe:large Large Level of Effort Project:Controls release_note:skip Skip the PR/issue when compiling release notes Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Dashboard Controls] Create State Storage for Control Group
5 participants