Fix template repeat for custom panels #188
Conversation
|
Levitate is-compatible report: 🔍 Resolving @grafana/data@latest... 🔬 Checking compatibility between ./src/module.ts and @grafana/data@9.5.1... 🔬 Checking compatibility between ./src/module.ts and @grafana/ui@9.5.1... 🔬 Checking compatibility between ./src/module.ts and @grafana/runtime@9.5.1... 🔬 Checking compatibility between ./src/module.ts and @grafana/e2e-selectors@9.5.1... ✔️ ./src/module.ts appears to be compatible with @grafana/data,@grafana/ui,@grafana/runtime,@grafana/e2e-selectors |
…les for the customInputActiveCamera display option
|
Backend code coverage report for PR #188 |
|
Frontend code coverage report for PR #188
|
| const tempVarName = getUrlTempVarName(displayOption || ''); | ||
| const tempVarVal = checkTempVar(displayOption); | ||
| const tempVarURLVal = queryParams[tempVarName]; | ||
| if (Array.isArray(tempVarURLVal) || tempVarVal !== tempVarURLVal || !(tempVarURLVal as string)) { |
There was a problem hiding this comment.
I could be misreading this, but if you're returning tempVarVal when tempVarVal !== tempVarURLVal aren't you functionally always returning tempVarVal?
There was a problem hiding this comment.
You're right I need to reevaluate this
There was a problem hiding this comment.
!(tempVarURLVal as string) can just be !tempVarURLVal
There was a problem hiding this comment.
Updated to just use the saved template var value tempVarVal as the default, otherwise check the URL for a value.
| const tempVarName = getUrlTempVarName(displayOption || ''); | ||
| const tempVarVal = checkTempVar(displayOption); | ||
| const tempVarURLVal = queryParams[tempVarName]; | ||
| if (Array.isArray(tempVarURLVal) || tempVarVal !== tempVarURLVal || !(tempVarURLVal as string)) { |
There was a problem hiding this comment.
!(tempVarURLVal as string) can just be !tempVarURLVal
| const tempVarName = getUrlTempVarName(displayOption || ''); | ||
| const tempVarVal = checkTempVar(displayOption); | ||
| const tempVarURLVal = queryParams[tempVarName]; | ||
| if (Array.isArray(tempVarURLVal) || tempVarVal !== tempVarURLVal || !(tempVarURLVal as string)) { |
There was a problem hiding this comment.
!(tempVarURLVal as string) can just be !tempVarURLVal
| entityId: entityId as string, | ||
| componentName: componentName as string, | ||
| kvsStreamName: kvsStreamName as string, |
There was a problem hiding this comment.
Can videoData handle these arguments if they're arrays or will it throw an error? Since getDisplayOptionValue returns a UrlQueryValue, it's possible it can also be one of these types string[] | number[] | boolean[]. But the VideoDataProps type expects only strings.
Some options for removing the type assertions from this file.
- Update
getDisplayOptionValueto only ever return a string. - Update videoData to support
UrlQueryValue, and update the type for the displayOptions state like so (the type can be extracted instead of being defined inline if needed)
const [displayOptions, setDisplayOptions] = useState<{
entityId: UrlQueryValue;
componentName: UrlQueryValue;
kvsStreamName: UrlQueryValue;
search: UrlQueryValue;
startTime: UrlQueryValue;
endTime: UrlQueryValue;
}>({
entityId: '',
componentName: '',
kvsStreamName: '',
search: search,
startTime: '',
endTime: '',
});There was a problem hiding this comment.
I believe UrlQueryValue can only be an array if the variable has a value and the string tempVarVal would be returned. The only way that UrlQueryValue is returned is if some label is in the URL that doesn't match an existing variable in a dashboard. This is possible through the TwinMaker plugin, but we don't support arrays.
There was a problem hiding this comment.
Updated getDisplayOptionValue to return a string. If the type cast gets wonky then there's something wrong with the dashboard setup anyway.
What this PR does / why we need it:
When a dashboard has a multi-value template variable in a row that repeats for each value of the variable, each value was not set correctly in a repeated Video Player panel.
The Scene Viewer panel has different types of display options. 3 of these options are for setting the values of certain variables based on a selection in the scene. On load the value of the variables are used to initialize the selection, but it should only be resolved from the URL values. Grafana gives users the ability to save default values which cause issues for the Scene Viewer when deselecting items in the scene.
1 option is for setting the default active camera which can be resolved by the URL or default dashboard value since it cannot be deselected.
Fixed by iwysiu
Which issue(s) this PR fixes:
Fixes #175
Fixes #109
Special notes for your reviewer: