feat: Add configurable query identifiers for Mixed Timeseries charts #34406
Conversation
- Add 'Show query identifiers' checkbox control to MixedTimeseries chart - Control appears above Rich tooltip in the Tooltip section - Default behavior: unchecked (no Query A/B identifiers shown) - When checked: adds (Query A) and (Query B) to legend and series names - Updates legend to use transformed series names instead of raw names - Conditional logic based on show_query_identifiers form data 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
The form control uses snake_case but gets converted to camelCase when passed to transform function
![korbit-ai[bot]](https://avatars.githubusercontent.com/in/322216?s=60&v=4){kind=small}
There was a problem hiding this comment.
I've completed my review and didn't find any issues.
Files scanned
| File Path | Reviewed |
|---|---|
| superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/types.ts | ✅ |
| superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/controlPanel.tsx | ✅ |
| superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts | ✅ |
Explore our documentation to understand the languages and file types we support and the files we ignore.
Check out our docs on how you can make Korbit work best for you and your team.
- Remove trailing whitespace - Fix ESLint formatting issues - Ensure code style consistency 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Add test for showQueryIdentifiers=false (no query identifiers in series names) - Add test for showQueryIdentifiers=true (query identifiers included in series names) - Remove debug console.log statements from transformProps.ts 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Add proper type casting for echartOptions.series in transformProps.test.ts - Ensures TypeScript compilation passes for CI checks 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Fix prettier formatting for map function in transformProps.test.ts - Ensures frontend linting checks pass 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
|
@yousoph Processing your ephemeral environment request here. Action: up. More information on how to use or configure ephemeral environments |
|
@yousoph Ephemeral environment spinning up at http://44.252.93.11:8080. Credentials are 'admin'/'admin'. Please allow several minutes for bootstrapping and startup. |
| }, | ||
| }, | ||
| ], | ||
| ...richTooltipSection.slice(1), // Skip the tooltip header since we added our own |
There was a problem hiding this comment.
Did claude add a tooltip header and then remove it, too?
There was a problem hiding this comment.
@yousoph can you confirm this works as expected? If not we should follow through with a quick PR to fix the feature
There was a problem hiding this comment.
It does function as expected but maybe this code needs to be fixed
My guess is it did something weird with the section header since the new control I added was at the top of the "Tooltip" section
…pache#34406) Co-authored-by: Claude <[email protected]>
SUMMARY
Adds a new configuration option "Show query identifiers" to Mixed Timeseries charts, allowing users to control whether "(Query A)" and "(Query B)" identifiers appear in tooltips.
Following the implementation of query identifiers in #33519, some customers provided feedback that they prefer not to see these identifiers in their charts. This PR makes the feature configurable.
Changes
Added "Show query identifiers" checkbox control in the Customize tab
Default behavior: identifiers are hidden
When enabled: shows the existing "(Query A)" and "(Query B)" behavior
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION