PanelEdit: Show when field options have override rules or data config that overrides the default #40250

torkelo · 2021-10-10T09:39:32Z

This was something that we wanted to do for v7 and v8 but we never had time to complete.

Shows a small info dot (orange) when data config is found for an option. The tooltip currently shows the data fields and config values, not sure if this really needed. Makes the tooltip a bit bigger.
Unit test for data config info dots
Show small info dot (blue) for override rules that override a default
Unit test for override rule info dots
Find a way to add filter action for each option that has are overridden by an override rule, the filter action would add the option name to option search so you would see the default and all override rules that contain that option and the option values. '

Issues

Find our Tooltip component's tooltips a bit hard to read, both the small text & the small padding/contrast/lack of border make them blend into the background too much.
Show data override config values or not? For some properties that have object values (data links, color scheme) it will just be json blob which just looks bad.

Todo

What colors to use for dots?
Feedback from ux team

Screenshots:
Unit has data override and No value has an override rule that overrides it

Options can have both data & override rule overrides

data override tooltip, not sure I showing values is needed? For some properties that have object values (data links, color scheme) it will just be json blob which just looks bad.

override rule info dot tooltip:

ryantxu · 2021-10-10T21:06:02Z

On a phone with low/no internet (so you may already do this!)... The case that would be really nice to advertise is when the datasource sets a value for unit, and the ui also sets unit. The one in the ui is just a 'default' so it is ignored

hugohaggmark

Did a shallow code review and found some small things.

I think using both orange and blue dot on the same prop looks a bit confusing, maybe we could have one a half orange/half blue dot instead of two?

public/app/features/dashboard/components/PanelEditor/OptionsPaneItemDescriptor.tsx

hugohaggmark · 2021-10-11T05:21:53Z

public/app/features/dashboard/components/PanelEditor/OptionsPaneItemDescriptor.tsx

+  renderOverrides() {
+    const { overrides } = this.props;
+    if (!overrides || overrides.length === 0) {
+      return;
+    }
+  }


This is probably an unfinished function?

or rather a relic, moved it out to OptionsPaneItemOverrides

hugohaggmark · 2021-10-11T05:22:33Z

public/app/features/dashboard/components/PanelEditor/OptionsPaneItemOverrides.tsx

+            style={{
+              backgroundColor: theme.colors[override.color].main,
+              width: 8,
+              height: 8,
+              borderRadius: '50%',
+              marginLeft: theme.spacing(1),
+              position: 'relative',
+              top: '-1px',
+            }}


Why the inline style?

original reason was emotions poor performance thinking the dots would have different colors, but think there is good reason to do this inline

torkelo · 2021-10-11T05:36:30Z

I think using both orange and blue dot on the same prop looks a bit confusing, maybe we could have one a half orange/half blue dot instead of two?

We could also use just blue dots, having different colors for the two cases might not be needed really, they both tell you the same thing, that the default is overwritten in some cases

dprokop · 2021-10-11T07:31:57Z

Found a small issue when one property has multiple overrides. There are as many dots being rendered as overrides:

If this is intentional, I would rather expect to be sth like this ⬤ x N?

dprokop · 2021-10-11T07:36:15Z

public/app/features/dashboard/components/PanelEditor/OptionsPaneItemOverrides.tsx

+  return (
+    <>
+      {overrides.map((override, index) => (
+        <Tooltip content={override.tooltip} key={index.toString()} placement="top">


Maybe a stupid ideas, but:

Would be great if the tooltip had an icon (magnifying glass?) that clicked would take the user directly to an override (if user defined).

Alternatively - not to show JSON blob - we could show the override editor for a given property so that the user could edit it if necessary straight away - but this may be overkill.

@dprokop
My idea was to have a magnifying glass or filter icon when clicked would add the property name to search, then you would see the default & all overrides that has this property.

Was thinking about this too. But don't you think people can find it confusing if we pre-fill the search input? I mean- if we pre-fill the search input then the user will have to figure out to clear it if he/she wants to get back to the full list. Maybe, when pre-filling the input, we need to blink the outline of the search input to indicate what has happened. But worth talking to UX peeps about this too as you planned.

torkelo · 2021-10-11T07:41:43Z

Found a small issue when one property has multiple overrides. There are as many dots being rendered as overrides:

think I am missing a break in the search for overrides loop :)

…-info-dots

Willena · 2021-10-12T22:18:14Z

I do like this feature :) Just some naive questions : What would happen if I have 20 (or more) series and override properties gets applied using regex ? Will this result in 20 or more colored dots ? Is the "overflow" of dots handled ?

…-info-dots

…neItemDescriptor.tsx Co-authored-by: Hugo Häggmark <[email protected]>

mckn · 2021-10-14T08:17:24Z

packages/grafana-ui/src/utils/useCombinedRefs.ts

+export function useCombinedRefs<T>(...refs: any) {
+  const targetRef = React.useRef<T>(null);
+
+  React.useEffect(() => {


Would it be possible to use a useMemo here instead? Or are we setting any of the refs in other useEffect hooks?

not sure, the other implementations of a hook like this I have seen used useEffect

…-info-dots

torkelo · 2021-10-28T07:57:54Z

@Willena only a single dot is shown if there is any override rule with that propery, same for the data override dot (if any field in the data have it set)

…-info-dots

torkelo · 2021-10-28T13:49:25Z

@ryantxu @dprokop @ryantxu Can you take a quick look a this again, think this is close to mergable in current form and want to merge it and we can iterate on it in further PRs.

dprokop · 2021-10-28T13:49:44Z

yup, sure

dprokop

Works OK and the code is good too. Can't find anything apart from the yarn pnp things that I think should be checked in in a separate PR.

public/app/features/dashboard/components/PanelEditor/OptionsPaneItemOverrides.tsx

hugohaggmark

Looks good and works as expected 🎉

hugohaggmark · 2021-10-29T05:38:33Z

public/app/features/dashboard/components/PanelEditor/OptionsPaneItemOverrides.tsx

+  'Some data fields have this option pre-configured. Add a field override rule to override the pre-configured value.';
+export const overrideRuleTooltipDescription = 'An override rule exists for this property';
+
+export function searchForOptionOverrides(


Maybe we should move these exported functions outside of the component and add tests?

…-info-dots

* main: (47 commits) Chore: Prevent loading error from showing too early (#41347) Chore: add context to login (#41316) Update dependency memoize-one to v6 (#41349) Docs: Added note that only string type is supported for query variables. (#41359) Alerting: fix bug where user is able to access rules from namespaces user is not part of (#41403) prometheus: enable new monaco-based query field (#41357) Chore: Go mod tidy (#41374) PanelEdit: Show when field options have override rules or data config that overrides the default (#40250) NodeGraph: Fix subTitle and secondaryStat being truncated in some cases (#40244) MarketTrend: add devenv dashboard (#41334) MarketTrend: add new alpha panel (#40909) StateTimeline: Share cursor with rest of the panels (#41038) Chore: cleanup ES query_builder test (#41360) Prometheus: Fix showing of errors (#41356) Update dependency lru-cache to v6 (#41327) api/ds/query: simplify data sources lookup for queries and expressions (#41172) A11y/UserAdminPage: Improves tab navigation and focus management (#41321) influxdb: improved explanation texts (#41351) prometheus: monaco: fix a corner-case with quotes (#41345) process app plugins first (#41346) ...

torkelo added 4 commits October 10, 2021 10:28

First pass at showing data override dots

dcc3843

Added test

3045e92

Adding override rule dots

9acb82f

Added unit test

edf28fe

torkelo requested review from hugohaggmark, ryantxu and dprokop October 10, 2021 09:39

torkelo requested a review from a team as a code owner October 10, 2021 09:39

torkelo requested review from kaydelaney and removed request for a team October 10, 2021 09:39

grafanabot added the area/frontend label Oct 10, 2021

hugohaggmark requested a review from a team October 11, 2021 05:16

hugohaggmark reviewed Oct 11, 2021

View reviewed changes

dprokop reviewed Oct 11, 2021

View reviewed changes

Merge remote-tracking branch 'origin/main' into field-overrides-label…

6313327

…-info-dots

torkelo added 2 commits October 13, 2021 13:53

Merge remote-tracking branch 'origin/main' into field-overrides-label…

fc3ff48

…-info-dots

Minor changes

c25be03

torkelo requested a review from a team October 13, 2021 13:39

Update public/app/features/dashboard/components/PanelEditor/OptionsPa…

7eae14a

…neItemDescriptor.tsx Co-authored-by: Hugo Häggmark <[email protected]>

mckn reviewed Oct 14, 2021

View reviewed changes

Merge remote-tracking branch 'origin/main' into field-overrides-label…

1918bed

…-info-dots

torkelo added 3 commits October 28, 2021 10:03

Merge remote-tracking branch 'origin/main' into field-overrides-label…

90fa77b

…-info-dots

Merge remote-tracking branch 'origin/main' into field-overrides-label…

2d47bd1

…-info-dots

Fixed ts issues

bd2189a

torkelo requested review from dprokop and hugohaggmark October 28, 2021 13:48

dprokop approved these changes Oct 28, 2021

View reviewed changes

kaydelaney reviewed Oct 28, 2021

View reviewed changes

public/app/features/dashboard/components/PanelEditor/OptionsPaneItemOverrides.tsx Outdated Show resolved Hide resolved

hugohaggmark approved these changes Oct 29, 2021

View reviewed changes

torkelo added 4 commits November 5, 2021 14:06

Merge remote-tracking branch 'origin/main' into field-overrides-label…

4a3ebc7

…-info-dots

Merge remote-tracking branch 'origin/main' into field-overrides-label…

f3a858f

…-info-dots

review feedback changes

b78c1f0

skipp broken e2e test

bcc4c3a

torkelo merged commit 3dee34c into main Nov 8, 2021

torkelo deleted the field-overrides-label-info-dots branch November 8, 2021 12:18

ashharrison90 mentioned this pull request Nov 15, 2021

Panel Edit: Implement change indicators for options #25430

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PanelEdit: Show when field options have override rules or data config that overrides the default #40250

PanelEdit: Show when field options have override rules or data config that overrides the default #40250

torkelo commented Oct 10, 2021

ryantxu commented Oct 10, 2021

hugohaggmark left a comment

hugohaggmark Oct 11, 2021

torkelo Oct 11, 2021

torkelo Oct 11, 2021

hugohaggmark Oct 11, 2021

torkelo Oct 11, 2021

torkelo commented Oct 11, 2021

dprokop commented Oct 11, 2021 •

edited

Loading

dprokop Oct 11, 2021

torkelo Oct 11, 2021

dprokop Oct 11, 2021

torkelo commented Oct 11, 2021

Willena commented Oct 12, 2021

mckn Oct 14, 2021

torkelo Oct 27, 2021

torkelo commented Oct 28, 2021

torkelo commented Oct 28, 2021

dprokop commented Oct 28, 2021

dprokop left a comment

hugohaggmark left a comment

hugohaggmark Oct 29, 2021

torkelo Nov 8, 2021

PanelEdit: Show when field options have override rules or data config that overrides the default #40250

PanelEdit: Show when field options have override rules or data config that overrides the default #40250

Conversation

torkelo commented Oct 10, 2021

ryantxu commented Oct 10, 2021

hugohaggmark left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

torkelo commented Oct 11, 2021

dprokop commented Oct 11, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

torkelo commented Oct 11, 2021

Willena commented Oct 12, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

torkelo commented Oct 28, 2021

torkelo commented Oct 28, 2021

dprokop commented Oct 28, 2021

dprokop left a comment

Choose a reason for hiding this comment

hugohaggmark left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dprokop commented Oct 11, 2021 •

edited

Loading