-
Notifications
You must be signed in to change notification settings - Fork 13.8k
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
refactor(standardized form data): refine interface and improve code smells #20518
Conversation
/testenv up |
Codecov Report
@@ Coverage Diff @@
## master #20518 +/- ##
==========================================
- Coverage 66.85% 66.85% -0.01%
==========================================
Files 1749 1751 +2
Lines 65415 65424 +9
Branches 6906 6910 +4
==========================================
+ Hits 43735 43740 +5
- Misses 19930 19934 +4
Partials 1750 1750
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
@zhaoyongjie Ephemeral environment spinning up at http://54.184.4.230:8080. Credentials are |
I think I found a bug when moving between Line Chart, Big Number and Pie chart. When I add 2 metrics while on Line Chart and then I switch to Big Number, it uses the first metric(correctly). Then I replace a metric on Big Number and switch to Pie chart. Pie Chart shows the second metric from Line chart instead of the metric from Big Number Screen.Recording.2022-06-28.at.11.43.54.mov |
Oh it actually looks like it rotates on the metrics available in standardized form data when I switch between charts with a single metric 😄 Screen.Recording.2022-06-28.at.11.49.09.mov |
Thanks! Nice catch! |
@@ -220,8 +220,4 @@ export function isDruidFormData( | |||
return 'granularity' in formData; | |||
} | |||
|
|||
export function isSavedMetric(metric: QueryFormMetric): metric is SavedMetric { |
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.
move to query/types/Metric.ts
/testenv up |
@zhaoyongjie Ephemeral environment spinning up at http://34.214.147.176:8080. Credentials are |
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.
Tested, works great!
Ephemeral environment shutdown and build artifacts deleted. |
SUMMARY
This PR intends to clean up code and refine interfaces. there are no functional changes.
denormalizeFormData
toformDataOverrides
in ControlPanelConfigupdateStandardizedState
in ControlPanelConfigsize
,entity
, andseries
toStandardizedControls
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A
TESTING INSTRUCTIONS
CI
ADDITIONAL INFORMATION