feat(dashboards): integrate text widget visualization into product#110245
feat(dashboards): integrate text widget visualization into product#110245nikkikapadia merged 15 commits intomasterfrom
Conversation
gggritso
left a comment
There was a problem hiding this comment.
Makes sense just a few nits!
gggritso
left a comment
There was a problem hiding this comment.
The new modal looks reasonable except I think it might have copied too much stuff from the regular modal?
|
|
||
| const HALF_CONTAINER_HEIGHT = 300; | ||
|
|
||
| const shouldWidgetCardChartMemo = (prevProps: any, props: any) => { |
There was a problem hiding this comment.
Hm, since this widget doesn't load any data, does it need memoization?
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
gggritso
left a comment
There was a problem hiding this comment.
Just a few more nits, almost good to go!
| <Stack gap="md"> | ||
| <Flex align="center" gap="sm"> | ||
| <Heading as="h3">{widget.title}</Heading> | ||
| </Flex> | ||
| </Stack> |
There was a problem hiding this comment.
Redundant Stack, I think? There's just one element
| direction="column" | ||
| height={`${HALF_CONTAINER_HEIGHT}px`} | ||
| position="relative" | ||
| paddingBottom="2xl" |
There was a problem hiding this comment.
This paddingBottom is a little suspicious, same with the Fragment around? Are these needs? Would be better to use a gap in the wrapper element to space things out
This PR integrates the text widget visualization into the
WidgetCardcomponent and theWidgetViewerModal. The option to select text widgets in the widget builder is not yet put in and will be my next PR.I have a dashboard where i've created a text widget with hacks that you can use to test this called "Text widget test".
Note: editing the widget will not work yet, this is just to get the visualization into the "viewing" surfaces of dashboards.
Closes DAIN-1319
Closes DAIN-1318