RN-1130: Update types for dashboard item presentation options #5380
Conversation
| granularity?: ValueOf<typeof GRANULARITIES>; | ||
| startDate?: Moment | string; | ||
| endDate?: Moment | string; | ||
| granularity?: `${VizPeriodGranularity}`; |
There was a problem hiding this comment.
Using a type like ${.....} makes the dev-x a little nicer because when you hover over the value it shows all the string values it could be
| const { viewType } = config; | ||
| if ( | ||
| viewType === 'multiValue' && | ||
| 'presentationOptions' in config && |
There was a problem hiding this comment.
It's a little annoying to have to type check for presentations every time they're used, and I did explore a util function but it ended up being not that useful since there are several different shapes of presentationOptions so I could never get the types exactly right in a generic function (mainly because if you have a union type and pass a key into a function as keyof the union type, it only gives you the common keys that apply to everything in that union type, not all the possible keys. Seemed more trouble than it was worth to make it work).
| @@ -176,7 +180,8 @@ const MatrixVisual = () => { | |||
| return ( | |||
| <> | |||
| <MatrixComponent | |||
| {...config} | |||
| // casting here because we know that the config is a MatrixConfig and it has a different shape than configs of other types, and while we know that this component only ever gets a MatrixConfig, the Matrix component doesn't know that as it all comes from the same context | |||
| {...(config as MatrixConfig)} | |||
There was a problem hiding this comment.
I didn't really want to have to cast the type here, but I wasn't sure of a better way, since all the config is from a common context that has a union type for config
| return { | ||
| ...reportData, | ||
| ...exportViewConfig, | ||
| } as ExportViewContent; // casting here to handle the different types of data that can be passed to the ExportViewContent component, which the union type doesn't cover well, and the fact that DashboardItemReport doesn't know when type is chart that it returns a ChartData type and not a ViewDataItem type etc |
There was a problem hiding this comment.
Not super stoked with having to cast here, but I was finding it really difficult when the data was different shapes, and the report type doesn't know that it is returning a chart report when type is chart, etc. Unsure if there is a better way?
For sure! I know, it's complicated, took me almost a week to wade through it all and get everything building 😓 |
There was a problem hiding this comment.
Thanks for making these changes @alexd-bes 🙏
All looks good to me!
Issue RN-1130: Update types for dashboard item presentation options
Changes:
typesfor dashboard item visualisations, mainly inpresentationOptions. Cross referenced what we had in the database with what we had in the codebaseui-chart-componentsandtupaia-webto use the correct types