-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
RN-1130: Update types for dashboard item presentation options #5380
Changes from 16 commits
690aa7a
320a3c4
c366962
0fc98fc
bf4495a
ccf2c9a
d7562bf
f3e09c0
efa354d
454a139
1af726f
79cba17
3a16ba8
ed89d6d
a9811e0
701f4c8
8d9d11e
b8ef593
2b5e139
82f573f
6c037cb
496b7c6
e9a19b1
ec74b14
2889ab6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,8 +9,8 @@ import { Moment } from 'moment'; | |
import { useParams } from 'react-router'; | ||
import { Typography } from '@material-ui/core'; | ||
import { getDefaultDates } from '@tupaia/utils'; | ||
import { MultiValueViewConfig } from '@tupaia/types'; | ||
import { DashboardItemConfig, DashboardItem as DashboardItemType } from '../../types'; | ||
import { DashboardItemConfig } from '@tupaia/types'; | ||
import { DashboardItem as DashboardItemType } from '../../types'; | ||
import { useReport } from '../../api/queries'; | ||
import { useDashboard } from '../Dashboard'; | ||
import { DashboardItemContent } from './DashboardItemContent'; | ||
|
@@ -50,17 +50,25 @@ const Title = styled(Typography).attrs({ | |
line-height: 1.4; | ||
`; | ||
|
||
const getShowDashboardItemTitle = (config: DashboardItemConfig, legacy?: boolean) => { | ||
const { presentationOptions, type, viewType, name } = config; | ||
const getShowDashboardItemTitle = (config?: DashboardItemConfig, legacy?: boolean) => { | ||
if (!config) return false; | ||
const { type, name } = config; | ||
if (!name) return false; | ||
if (viewType === 'multiValue') { | ||
// if report is legacy, show title because it won't have the config set | ||
return ( | ||
(presentationOptions as MultiValueViewConfig['presentationOptions'])?.isTitleVisible || legacy | ||
); | ||
if (type === 'component') return false; | ||
if (type === 'view') { | ||
const { viewType } = config; | ||
if ( | ||
viewType === 'multiValue' && | ||
'presentationOptions' in config && | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
config.presentationOptions | ||
) { | ||
const { presentationOptions } = config; | ||
// if report is legacy, show title because it won't have the config set | ||
return presentationOptions?.isTitleVisible || legacy; | ||
} | ||
if (viewType?.includes('Download') || viewType === 'multiSingleValue') return false; | ||
} | ||
if (viewType?.includes('Download') || type === 'component' || viewType === 'multiSingleValue') | ||
return false; | ||
|
||
return true; | ||
}; | ||
|
||
|
@@ -92,7 +100,7 @@ export const DashboardItem = ({ dashboardItem }: { dashboardItem: DashboardItemT | |
legacy: dashboardItem?.legacy, | ||
}); | ||
|
||
const { config = {}, legacy } = dashboardItem; | ||
const { config, legacy } = dashboardItem; | ||
|
||
const showTitle = getShowDashboardItemTitle(config, legacy); | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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