[Embeddable Rebuild] [Controls] Refactor control editing + creation #187606
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
added
the
release_note:skip
Heenawter
force-pushed
the
embeddableRebuild_controls_add-creation_2024-07-03
branch
from
22044c0 to
609a2f2
Compare
|
/ci |
|
/ci |
Heenawter
commented
Jul 8, 2024
Comment on lines
+16
to
+22
| .controlPanel--label { | ||
| @include euiTextTruncate; | ||
| background-color: transparent; | ||
| border-radius: $euiBorderRadius; | ||
| margin-left: 0 !important; | ||
| padding-left: 0 !important; | ||
| } |
There was a problem hiding this comment.
This fixes a style error where the truncation was happening before the max-width applied, so labels were getting cut off too early on the "small" control size. I also verified that the truncation now happens at the same point that it does for the old controls, so there should be no visual changes from the user's perspective once the React controls are officially swapped in Dashboard.
| Before | After | Legacy |
|---|---|---|
 |
 |
 |
 |
 |
 |
Heenawter
commented
Jul 8, 2024
Comment on lines
+150
to
+157
| <EuiToolTip | ||
| anchorClassName="controlPanel--labelWrapper" | ||
| content={panelTitle || defaultPanelTitle} | ||
| > | ||
| <EuiFormLabel className="controlPanel--label"> | ||
| {panelTitle || defaultPanelTitle} | ||
| </EuiFormLabel> | ||
| </EuiToolTip> |
There was a problem hiding this comment.
I accidentally forgot to include the tooltip when initially building this, but we need it for when things get truncated.
Heenawter
commented
Jul 8, 2024
| @@ -22,6 +22,7 @@ | |||
| } | |||
|
|
|||
| .rangeSliderAnchor__fieldNumber { | |||
| min-width: auto !important; | |||
There was a problem hiding this comment.
This fixes a bug where the step arrows were being cut off on the small size:
| Before | After |
|---|---|
 |
 |
…ld_controls_add-creation_2024-07-03
…github.com:heenawter/kibana into embeddableRebuild_controls_add-creation_2024-07-03
|
/ci |
Heenawter
commented
Jul 8, 2024
| CustomOptionsComponent: ({ stateManager, setControlEditorValid }) => { | ||
| const step = useStateFromPublishingSubject(stateManager.step); | ||
| CustomOptionsComponent: ({ initialState, updateState, setControlEditorValid }) => { | ||
| const [step, setStep] = useState(initialState.step ?? 1); |
There was a problem hiding this comment.
The default to 1 needs to happen here too, specifically for creation.
|
/ci |
nreese
reviewed
Jul 8, 2024
nreese
reviewed
Jul 8, 2024
examples/controls_example/public/react_controls/data_controls/data_control_editor.test.tsx
Outdated
Show resolved
Hide resolved
nreese
reviewed
Jul 8, 2024
examples/controls_example/public/react_controls/data_controls/initialize_data_control.tsx
Outdated
Show resolved
Hide resolved
nreese
reviewed
Jul 15, 2024
examples/controls_example/public/react_controls/data_controls/initialize_data_control.tsx
Outdated
Show resolved
Hide resolved
nreese
approved these changes
Jul 15, 2024
ryankeairns
approved these changes
Jul 17, 2024
…ld_controls_add-creation_2024-07-03
kibanamachine
added
v8.16.0
backport:skip
💛 Build succeeded, but was flaky
Failed CI StepsMetrics [docs]
History
cc @Heenawter |
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.
Closes #187504
Summary
This PR changes how control creation/editing is handled in the new system by changing from a state manager to an explicit
initialState+updateStatecallback structure. Unfortunately, thestateManagersystem really only works if we had inline creation for controls - since we don't have that, the embeddable (and hence thestateManager) doesn't exist until after the editor is saved. Therefore, especially with respect to the custom options component, we had no way of knowing what keys thestateManagershould include when trying to create a new control. The new system isn't quite as clean IMO, but it works better for our current goals with this refactor. We can revisit thestateManageridea once controls support inline editing 👍This PR also fixes a few visual bugs, noted below.
Checklist
For maintainers