-
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
feat: drill by display chart #23524
feat: drill by display chart #23524
Conversation
superset-frontend/src/components/Chart/DrillBy/DrillByChart.tsx
Outdated
Show resolved
Hide resolved
superset-frontend/src/components/Chart/DrillBy/DrillByChart.tsx
Outdated
Show resolved
Hide resolved
superset-frontend/src/components/Chart/DrillBy/DrillByChart.tsx
Outdated
Show resolved
Hide resolved
superset-frontend/src/components/Chart/DrillBy/DrillByChart.tsx
Outdated
Show resolved
Hide resolved
5b3c1a5
to
911f7e0
Compare
superset-frontend/src/components/Chart/DrillBy/DrillByChart.tsx
Outdated
Show resolved
Hide resolved
superset-frontend/src/components/Chart/DrillBy/DrillByChart.tsx
Outdated
Show resolved
Hide resolved
superset-frontend/src/components/Chart/DrillBy/DrillByChart.tsx
Outdated
Show resolved
Hide resolved
superset-frontend/src/components/Chart/DrillBy/DrillByChart.tsx
Outdated
Show resolved
Hide resolved
> | ||
{chartDataResult ? ( | ||
<SuperChart | ||
behaviors={[Behavior.INTERACTIVE_CHART]} |
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.
INTERACTIVE_CHART
is for cross filters, and in the drill by modal we only want to support further drill by... but let's leave it for a separate pr to clean up 🙂
d9a0e9b
to
8b684d1
Compare
Codecov Report
@@ Coverage Diff @@
## master #23524 +/- ##
==========================================
- Coverage 67.68% 67.62% -0.07%
==========================================
Files 1914 1916 +2
Lines 73962 74009 +47
Branches 8029 8038 +9
==========================================
- Hits 50064 50046 -18
- Misses 21852 21913 +61
- Partials 2046 2050 +4
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 6 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
'glob:*api/v1/chart/data?form_data=%7B%22slice_id%22%3A18%7D'; | ||
|
||
const chart = chartQueries[sliceId]; | ||
// const fetchWithData = () => { |
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.
Could you remove commented code?
expect(await screen.findByText('No Results')).toBeInTheDocument(); | ||
}); | ||
|
||
// test('should render SuperChart', async () => { |
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.
Should this test be fixed/uncommented before merging or should it be removed?
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.
I couldn't get the test to work. i will remove it
SUMMARY
BEFORE/AFTER SCREENSHOTS OR ANIMATE
Screen.Recording.2023-03-31.at.11.10.10.AM.mov
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION