Allow overriding display settings of indicators in mdims #4460
Conversation
rakyi
force-pushed
the
mdim-display-override
branch
from
719bf31 to
8aa0683
Compare
We now normalize the indicators to a list of objects, which can have an optional `display` property. See `IndicatorConfig`.
rakyi
force-pushed
the
mdim-display-override
branch
from
8aa0683 to
646349a
Compare
|
Quick links (staging server):
Login:
SVG tester:Number of differences (default views): 0 ✅ Edited: 2025-01-16 11:07:03 UTC |
|
|
||
| type IndicatorConfigBeforePreProcessing = Omit<IndicatorConfig, "id"> & { | ||
| // Indicator ID or catalog path | ||
| id: number | string |
There was a problem hiding this comment.
I'm wondering if now, we should be using catalogPath here specifically when referring to these variables by path?
In the past, I opted to have them as a union type precisely because it was a flat property and not an object. But now that we have this as an object either way, this all doesn't apply any more, and maybe the logic gets easier if we don't have to have so many type-based conditionals on it?
Ideally, we'd want to have a union type, like
{ id: number } | { catalogPath: string }
adminSiteServer/multiDim.ts
Outdated
| case "object": { | ||
| if (isIndicatorConfig(indicator)) return indicator | ||
| if (typeof indicator.id === "string") { | ||
| const id = catalogPathToIndicatorIdMap.get(indicator.id) |
There was a problem hiding this comment.
I think you'll need to change the definition of allCatalogPaths above also, so that catalogPathToIndicatorIdMap actually contains the correct mapping for when indicator is an object.
There was a problem hiding this comment.
Nice catch, I was relying too much on type checking. Fixed.
We now normalize the indicators to a list of objects, which can have an optional
displayproperty. SeeIndicatorConfig.Resolves #4040