Migrate Query and config editors to new form styling under feature toggle #255
Conversation
| /> | ||
| ); | ||
|
|
||
| const selectEl = screen.getByRole('combobox', { name: selectors.components.ConfigEditor.ClusterID.input }); |
There was a problem hiding this comment.
I think the old test was selecting the wrong Cluster identifier input. As it was set up, cluser Id Select shouldn't have been shown, instead the cluster disabled autofilled field should have.
I added a prop to get it to show, and a selector for the combobox in particular (not the input).
I could be getting this wrong, but I think it should be ok now. I double checked #228 to verify the logic.
| <EditorField | ||
| width={15} | ||
| label={selectors.components.ConfigEditor.schema.input} | ||
| tooltip="Use the selected schema with the $__schema macro" |
There was a problem hiding this comment.
For some reason, description prop in EditorField doesn't show up so Im leaving tooltips for now until we can investigate.
grafana/grafana-experimental#89
There was a problem hiding this comment.
the underlying field component in @grafana/ui renders the description in the label only if the label is a string
https://github.com/grafana/grafana/blob/bf8af608a7bda9a287071a3f4bdbc96d9d3d335f/packages/grafana-ui/src/components/Forms/Field.tsx#L95-L102
but the editor field component passes in a label element so the description doesn't show up.
There was a problem hiding this comment.
Indeed, thanks for getting to the bottom of this @kevinwcyu! Added this to the ticket :)
| /> | ||
| </Label> | ||
| ); | ||
| } | ||
|
|
||
| const getStyles = (theme: GrafanaTheme2) => ({ |
There was a problem hiding this comment.
These adjustments don't change the old form, only the new form with width adjustments.
| }); | ||
| } | ||
| run('ConfigEditor with new form styling disabled'); | ||
| config.featureToggles.awsDatasourcesNewFormStyling = true; |
There was a problem hiding this comment.
in line 57 this toggle is already set to true. also not sure whether this will have an effect since @grafana/runtime is mocked above?
There was a problem hiding this comment.
Ok so there indeed is a problem with these tests, and TIL about Jest's setup and teardown lifecycle 👩🏻🎓
Since jest "executes all describe handlers in a test file before it executes any of the actual tests", the value of the feature toggle will always be true (or whatever value we put last), since we overwrite it globally.
The only way I found to circumvent this is to separate "toggle disabled" and "toggle enabled" describe blocks and add
beforeAll(() => {
valueOfTheToggle = whatever
})
inside each block. Maybe there's a better way that I can't see right now (or just copying the tests lol)🤔
| <EditorField | ||
| width={15} | ||
| label={selectors.components.ConfigEditor.schema.input} | ||
| tooltip="Use the selected schema with the $__schema macro" |
There was a problem hiding this comment.
the underlying field component in @grafana/ui renders the description in the label only if the label is a string
https://github.com/grafana/grafana/blob/bf8af608a7bda9a287071a3f4bdbc96d9d3d335f/packages/grafana-ui/src/components/Forms/Field.tsx#L95-L102
but the editor field component passes in a label element so the description doesn't show up.
idastambuk
changed the title
idastambuk
force-pushed
the
287-redshift-queryeditor-and-config-editor-migration
branch
from
d07e03f
to
2bd4123
Compare
idastambuk
deleted the
287-redshift-queryeditor-and-config-editor-migration
branch
Migrating Redshift Config Editor to the new design system form components, as well as the query editor to use EditorField components from grafana/experimental.
The new components are under a Grafana feature flag.
Config Editor
What was done
Before & After
Query Editor
What was done:
Format assection in a Collapse as a temporary solution. Added a ticket here to mimic what we have in Prometheus and Loki, so when that gets picked up I will update here.Before & After
Note: I also had to update plugin tools in the last commit, as after the dependency upgrades the plugin had some errors.