grafana / iot-sitewise-datasource

IoT Sitewise
Apache License 2.0
19 stars 9 forks source link

Query and Config editors: Migrate to new form styling under feature toggle #244

Closed idastambuk closed 8 months ago

idastambuk commented 8 months ago

What this PR does / why we need it: Migrating 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 awsDatasourcesNewFormStyling.

Config Editor

What was done Replace the editor inline fields for the grafana/ui Field components if the feature flag is enabled

Screenshot 2023-11-06 at 2 13 39 PM Screenshot 2023-11-06 at 2 13 17 PM

Query Editor

What was done

Which issue(s) this PR fixes:

Fixes https://github.com/grafana/oss-plugin-partnerships/issues/634

Special notes for your reviewer:

idastambuk commented 8 months ago

Before selecting an asset, there's an empty row. let's hide rendering that until the necessary information is required to render what should go in that row.

Screenshot 2023-11-06 at 11 18 17 AM

Great catch, just committed a change in logic that uses a new flag for the new form options row

idastambuk commented 8 months ago

So much good work here! I'm curious if you think we can break this up into smaller prs at all? For example I see some good work here to refactor from a class component to a functional component, could that be done in a separate pr first, then we move to the styling? Or maybe tests in their own pr? Or the logic change in types.ts, is that directly tied to this new form styling? Curious what your thoughts are as I haven't dived into it too much!

Most of the changes are tied to the restyling, so it won't be easy to separate them. The only one that isn't, is changing the component to a functional one, but that's just one component and I thought it would be an easy win (and not really a lot of code changes) so I slipped it in this one. A possible solution to make this easier to review is to separate the restyling into Config and Query editor PRs, do you think it's worth it at this point?