-
Notifications
You must be signed in to change notification settings - Fork 13.9k
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
fix(altered-modal): displayed the metric value in altered modal correctly #18813
fix(altered-modal): displayed the metric value in altered modal correctly #18813
Conversation
Hello @prosdev0107 thanks for the PR. There are some linting issues to fix. Also, I don't think you need to tick the "Changes UI" checkbox in the PR description as that is intended for changes that affect the UI, in this case, this is just a fix of standard behavior. |
Agreed on the checkbox, though I appreciate over-checking rather than under-checking :) To run the linter locally, just run |
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.
LGTM
Approved and waiting for CI. As with all fix PRs, it'd be great if there were a test. Let me know if that looks like a reasonably easy addition here. |
Codecov Report
@@ Coverage Diff @@
## master #18813 +/- ##
=======================================
Coverage 66.31% 66.31%
=======================================
Files 1620 1620
Lines 63075 63090 +15
Branches 6370 6375 +5
=======================================
+ Hits 41827 41837 +10
- Misses 19591 19594 +3
- Partials 1657 1659 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
….com/prosdev0107/superset into fix/36630-altered-modal-metrics-show
SUMMARY
Altered Modal Doesn't Show Metrics Properly
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Before:
After:
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION