[Embeddable Rebuild] [Controls] Clean up services + TODOs #193180
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Heenawter
added
Team:Presentation
Heenawter
force-pushed
the
embeddableRebuild_controls_cleanup-todos-and-services_2024-09-12
branch
from
6269f45 to
c71bfeb
Compare
Heenawter
commented
Sep 17, 2024
| @@ -15,8 +15,6 @@ import { OptionsListSortingType } from './suggestions_sorting'; | |||
| import { DefaultDataControlState } from '../types'; | |||
| import { OptionsListSearchTechnique } from './suggestions_searching'; | |||
|
|
|||
| export const OPTIONS_LIST_CONTROL = 'optionsListControl'; // TODO: Replace with OPTIONS_LIST_CONTROL_TYPE | |||
There was a problem hiding this comment.
After #192017, this was not actually used - so removing it now.
| allowExpensiveQueries: boolean; | ||
| // TODO: Rename this route as part of https://github.com/elastic/kibana/issues/174961 | ||
| }>('/internal/controls/optionsList/getExpensiveQueriesSetting', { | ||
| }>('/internal/controls/getExpensiveQueriesSetting', { |
There was a problem hiding this comment.
I renamed this route since it's no longer specific to the options list - the control group now does the fetching.
| // HasSerializableState types serializeState as function returning 'MaybePromise' | ||
| // Controls serializeState is sync | ||
| serializeState: () => SerializedPanelState<DefaultControlState>; | ||
| /** TODO: Make these non-public as part of https://github.com/elastic/kibana/issues/174961 */ |
There was a problem hiding this comment.
I decided there is no reason to lock down the API at this point in time, especially since we are no longer allowing external plugins to register their own control types. It honestly felt like overcomplicating to split this into "public" and "private" APIs 😅 So I just removed this TODO
| @@ -62,7 +62,6 @@ export type ControlApiInitialization<ControlApi extends DefaultControlApi = Defa | |||
| 'serializeState' | 'getTypeDisplayName' | 'clearSelections' | |||
| >; | |||
|
|
|||
| // TODO: Move this to the Control plugin's setup contract | |||
There was a problem hiding this comment.
We decided to not allow registering controls via the Control plugin's setup contract, so I removed this TODO
Heenawter
commented
Sep 17, 2024
| @@ -113,14 +111,15 @@ export const getRangesliderControlFactory = ( | |||
|
|
|||
| const dataLoadingSubscription = combineLatest([loadingMinMax$, loadingHasNoResults$]) | |||
| .pipe( | |||
| debounceTime(100), | |||
There was a problem hiding this comment.
I added a small debounce here to prevent the loading state from "flashing" as often as it did before.
|
Pinging @elastic/kibana-presentation (Team:Presentation) |
nreese
reviewed
Sep 18, 2024
...blic/react_controls/controls/data_controls/range_slider/get_range_slider_control_factory.tsx
Outdated
Show resolved
Hide resolved
nreese
approved these changes
Sep 18, 2024
…-services_2024-09-12
💚 Build Succeeded
Metrics [docs]Module Count
Public APIs missing comments
Async chunks
Page load bundle
Unknown metric groupsAPI count
async chunk count
References to deprecated APIs
History
To update your PR or re-run it, just comment with: cc @Heenawter |
rudolf
approved these changes
Sep 19, 2024
kibanamachine
pushed a commit
to kibanamachine/kibana
that referenced
this pull request
Sep 19, 2024
…3180) Part of elastic#192005 Closes elastic#167438 ## Summary ## Summary This PR represents the second major cleanup task for the control group embeddable refactor. The tasks included in this PR can be loosely summarized as follows: 1. It removes the old, cumbersome services implementation and replaces it with a much simpler system (which is the same one used in the `links` + `presentation_panel` plugins). - This it the main reason for the decrease in lines - the old system required a **huge** amount of boilerplate code, which is no longer necessary with the new method for storing services. 2. It addresses and/or removes any remaining TODO comments in the `controls` plugin - This includes renaming `ControlStyle` and `DEFAULT_CONTROL_STYLE` to `ControlLabelPosition` and `DEFAULT_CONTROL_LABEL_POSITION` respectively, which represents a bulk of the changes. 3. It moves all compatibility checks for all control actions to be async imported. 4. It removes the ability to register controls from the `controls` plugin setup contract. ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios ### For maintainers - [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) --------- Co-authored-by: kibanamachine <[email protected]> (cherry picked from commit db55574)
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
Heenawter
deleted the
embeddableRebuild_controls_cleanup-todos-and-services_2024-09-12
branch
kibanamachine
added a commit
that referenced
this pull request
Sep 19, 2024
) (#193429) # Backport This will backport the following commits from `main` to `8.x`: - [[Embeddable Rebuild] [Controls] Clean up services + TODOs (#193180)](#193180) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Hannah Mudge","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-09-19T14:02:49Z","message":"[Embeddable Rebuild] [Controls] Clean up services + TODOs (#193180)\n\nPart of https://github.com/elastic/kibana/issues/192005\r\nCloses https://github.com/elastic/kibana/issues/167438\r\n\r\n## Summary\r\n\r\n\r\n## Summary\r\n\r\nThis PR represents the second major cleanup task for the control group\r\nembeddable refactor. The tasks included in this PR can be loosely\r\nsummarized as follows:\r\n1. It removes the old, cumbersome services implementation and replaces\r\nit with a much simpler system (which is the same one used in the `links`\r\n+ `presentation_panel` plugins).\r\n- This it the main reason for the decrease in lines - the old system\r\nrequired a **huge** amount of boilerplate code, which is no longer\r\nnecessary with the new method for storing services.\r\n2. It addresses and/or removes any remaining TODO comments in the\r\n`controls` plugin\r\n- This includes renaming `ControlStyle` and `DEFAULT_CONTROL_STYLE` to\r\n`ControlLabelPosition` and `DEFAULT_CONTROL_LABEL_POSITION`\r\nrespectively, which represents a bulk of the changes.\r\n3. It moves all compatibility checks for all control actions to be async\r\nimported.\r\n4. It removes the ability to register controls from the `controls`\r\nplugin setup contract.\r\n\r\n### Checklist\r\n\r\n- [x] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n\r\n\r\n### For maintainers\r\n\r\n- [ ] This was checked for breaking API changes and was [labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine <[email protected]>","sha":"db5557429979b9a0f3420a50a06c7fd69cbdf5b2","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:Presentation","release_note:skip","impact:high","v9.0.0","backport:prev-minor"],"title":"[Embeddable Rebuild] [Controls] Clean up services + TODOs","number":193180,"url":"https://github.com/elastic/kibana/pull/193180","mergeCommit":{"message":"[Embeddable Rebuild] [Controls] Clean up services + TODOs (#193180)\n\nPart of https://github.com/elastic/kibana/issues/192005\r\nCloses https://github.com/elastic/kibana/issues/167438\r\n\r\n## Summary\r\n\r\n\r\n## Summary\r\n\r\nThis PR represents the second major cleanup task for the control group\r\nembeddable refactor. The tasks included in this PR can be loosely\r\nsummarized as follows:\r\n1. It removes the old, cumbersome services implementation and replaces\r\nit with a much simpler system (which is the same one used in the `links`\r\n+ `presentation_panel` plugins).\r\n- This it the main reason for the decrease in lines - the old system\r\nrequired a **huge** amount of boilerplate code, which is no longer\r\nnecessary with the new method for storing services.\r\n2. It addresses and/or removes any remaining TODO comments in the\r\n`controls` plugin\r\n- This includes renaming `ControlStyle` and `DEFAULT_CONTROL_STYLE` to\r\n`ControlLabelPosition` and `DEFAULT_CONTROL_LABEL_POSITION`\r\nrespectively, which represents a bulk of the changes.\r\n3. It moves all compatibility checks for all control actions to be async\r\nimported.\r\n4. It removes the ability to register controls from the `controls`\r\nplugin setup contract.\r\n\r\n### Checklist\r\n\r\n- [x] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n\r\n\r\n### For maintainers\r\n\r\n- [ ] This was checked for breaking API changes and was [labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine <[email protected]>","sha":"db5557429979b9a0f3420a50a06c7fd69cbdf5b2"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/193180","number":193180,"mergeCommit":{"message":"[Embeddable Rebuild] [Controls] Clean up services + TODOs (#193180)\n\nPart of https://github.com/elastic/kibana/issues/192005\r\nCloses https://github.com/elastic/kibana/issues/167438\r\n\r\n## Summary\r\n\r\n\r\n## Summary\r\n\r\nThis PR represents the second major cleanup task for the control group\r\nembeddable refactor. The tasks included in this PR can be loosely\r\nsummarized as follows:\r\n1. It removes the old, cumbersome services implementation and replaces\r\nit with a much simpler system (which is the same one used in the `links`\r\n+ `presentation_panel` plugins).\r\n- This it the main reason for the decrease in lines - the old system\r\nrequired a **huge** amount of boilerplate code, which is no longer\r\nnecessary with the new method for storing services.\r\n2. It addresses and/or removes any remaining TODO comments in the\r\n`controls` plugin\r\n- This includes renaming `ControlStyle` and `DEFAULT_CONTROL_STYLE` to\r\n`ControlLabelPosition` and `DEFAULT_CONTROL_LABEL_POSITION`\r\nrespectively, which represents a bulk of the changes.\r\n3. It moves all compatibility checks for all control actions to be async\r\nimported.\r\n4. It removes the ability to register controls from the `controls`\r\nplugin setup contract.\r\n\r\n### Checklist\r\n\r\n- [x] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n\r\n\r\n### For maintainers\r\n\r\n- [ ] This was checked for breaking API changes and was [labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine <[email protected]>","sha":"db5557429979b9a0f3420a50a06c7fd69cbdf5b2"}}]}] BACKPORT--> Co-authored-by: Hannah Mudge <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Part of #192005
Closes #167438
Summary
Summary
This PR represents the second major cleanup task for the control group embeddable refactor. The tasks included in this PR can be loosely summarized as follows:
links+presentation_panelplugins).controlspluginControlStyleandDEFAULT_CONTROL_STYLEtoControlLabelPositionandDEFAULT_CONTROL_LABEL_POSITIONrespectively, which represents a bulk of the changes.controlsplugin setup contract.Checklist
For maintainers