AdhocFilters: Show label for selected key

grafana / scenes

Build Grafana dashboards directly in your Grafana app plugins.

https://grafana.com/developers/scenes

Apache License 2.0

141 stars 21 forks source link

AdhocFilters: Show label for selected key #690

Closed ashharrison90 closed 6 months ago

ashharrison90 commented 7 months ago

Ensures we find the selected option from the list and get the full key/value object instead of creating a new object from just the key

📦 Published PR as canary version: 4.11.3--canary.690.8783271240.0

:sparkles: Test out this PR locally via: ```bash npm install @grafana/scenes@4.11.3--canary.690.8783271240.0 # or yarn add @grafana/scenes@4.11.3--canary.690.8783271240.0 ```

dprokop commented 7 months ago

@ashharrison90 - think this ain't sufficient, as it will still use key.value when it's synced from URL. Same with the dimension values, which are affected by the same issue.

I think we need to trigger value "validation" on acivation when there are filters that were synced from URL. For that we need to fetch the keys from the data source and upate the available options accordingly.

@torkelo - any reason why we don't do that already?

torkelo commented 7 months ago

AdHocFilters are not really well designed for "display labels" for keys, as we only include the value in URL and loading the filter state from URL should be all that is needed before queries are issued and state sync

Do not want the AdHocFilterRenderer to have to issue a query for every filter to lookup the label, or would be nice to avoid that, just so messy

bfmatei commented 7 months ago

@torkelo @dprokop @ashharrison90

I just pushed an iteration of syncing the label via the URL. It works like this:

the label is added to the URL by using a comma separator between the value and label
we escape the commas in the label/value using __gfc__ similarly to what we do for the pipe
if the key and label are different, we just omit the duplication

One thing to have in mind: extending AdHocFilter internally and adding the labels there seemed the best solution. Alternatively we can have some mappings in the AdHocFiltersVariableState. Let me know if you prefer the latter or you have another idea on how to achieve it.

Also a small demo:

https://github.com/grafana/scenes/assets/9215315/fe2bdbce-0a3e-40ac-8cc9-179ed28629b7

grafanabot commented 6 months ago

:rocket: PR was released in v4.11.3 :rocket: