feat(dashboard): chart customization/dynamic group by in dashboards #33831
Conversation
|
Based on your review schedule, I'll hold off on reviewing this PR until it's marked as ready for review. If you'd like me to take a look now, comment
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #33831 +/- ##
===========================================
+ Coverage 60.48% 72.88% +12.39%
===========================================
Files 1931 559 -1372
Lines 76236 40403 -35833
Branches 8568 4245 -4323
===========================================
- Hits 46114 29449 -16665
+ Misses 28017 9857 -18160
+ Partials 2105 1097 -1008
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Co-authored-by: GitHub Action <[email protected]>
Co-authored-by: GitHub Action <[email protected]>
Co-authored-by: GitHub Action <[email protected]>
Co-authored-by: GitHub Action <[email protected]>
Co-authored-by: GitHub Action <[email protected]>
|
|
||
| function getAffectedChartIdsFromCustomization( | ||
| chartCustomization: ChartCustomizationItem[], | ||
| state: any, |
There was a problem hiding this comment.
Curious if there is a better type definition that we can re-use for this
There was a problem hiding this comment.
yes I will be fixing this.
|
|
||
| const updateDashboard = makeApi< | ||
| Partial<DashboardInfo>, | ||
| { result: Partial<DashboardInfo>; last_modified_time: number } |
There was a problem hiding this comment.
It seems this was defined before as well, can we dry up?
| chartId: number; | ||
| } | ||
|
|
||
| const StyledGroupByCount = styled.div` |
There was a problem hiding this comment.
It seems like Ant Design might offer something like this that we can re-use with minimal style customizations if any at all?
| const MIN_WIDTH = 880; | ||
| const MODAL_MARGIN = 16; | ||
|
|
||
| const StyledModalWrapper = styled(StyledModal)<{ expanded: boolean }>` |
There was a problem hiding this comment.
@LevisNgigi wasn't the plan to re-use the existing native filters modal and form instead of creating two separate components that almost do the same thing? Wouldn't composability help here?
feat(dashboard): chart customization/dynamic group by in dashboards
SUMMARY
WIP
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION