Repository navigation
feat(a11y): enable ECharts aria-label - #28171
Conversation
frassinier
changed the title
frassinier
changed the title
john-bodley
added
the
review:checkpoint
john-bodley
changed the title
|
I'm not sure we want to turn on the patterns by default. A lot of people don't want/need them, and this provides no option to disable them. We could, of course, add a chart control, but I don't think that's much better. I think the right approach might be to wait until we have this SIP in effect, so a user can choose to apply this across the board for charts/dashboards they view/edit/create. |
|
@rusackas, I agree! but what do you think of just adding the automated |
john-bodley
removed
the
review:checkpoint
|
Sorry this slid under the radar. I think we're all for aria-label additions, and we can review the idea of the patterns when we have the User Preferences feature (currently in SIP process). This needs a rebase, if you don't mind doing so... hopefully we can get it across the finish line! |
|
@rusackas, all set! |
|
I think for now, whilst we await the User Preference SIP, we should just add this as a config flag so admins can turn it on and off. Would love to get this merged if we can do so :) |
![bito-code-review[bot]](https://avatars.githubusercontent.com/in/1061978?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Agent Run #a6ac45
Actionable Suggestions - 1
-
superset-frontend/plugins/plugin-chart-echarts/src/components/Echart.tsx - 1
- Aria configuration misplaced · Line 202-210
Review Details
-
Files reviewed - 1 · Commit Range:
4914b99..2f31970- superset-frontend/plugins/plugin-chart-echarts/src/components/Echart.tsx
-
Files skipped - 0
-
Tools
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
Bito Usage Guide
Commands
Type the following command in the pull request comment and save the comment.
-
/review- Manually triggers a full AI review. -
/pause- Pauses automatic reviews on this pull request. -
/resume- Resumes automatic reviews. -
/resolve- Marks all Bito-posted review comments as resolved. -
/abort- Cancels all in-progress reviews.
Refer to the documentation for additional commands.
Configuration
This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.
Documentation & Help
AI Code Review powered by
| chartRef.current.setOption( | ||
| { | ||
| aria: { | ||
| enabled: true, | ||
| }, | ||
| ...echartOptions, | ||
| }, | ||
| true, | ||
| ); |
There was a problem hiding this comment.
The aria configuration is placed in a separate setOption call that gets overridden by the subsequent themed setOption during normal rendering (when not refreshing), preventing aria from being enabled. Move the aria setting into the base theme to ensure it's included in the final options.
Code suggestion
Check the AI-generated fix before applying
--- superset-frontend/plugins/plugin-chart-echarts/src/components/Echart.tsx
+++ superset-frontend/plugins/plugin-chart-echarts/src/components/Echart.tsx
@@ -190,15 +190,6 @@
Object.entries(zrEventHandlers || {}).forEach(([name, handler]) => {
chartRef.current?.getZr().off(name);
chartRef.current?.getZr().on(name, handler);
});
-
- chartRef.current.setOption(
- {
- aria: {
- enabled: true,
- },
- ...echartOptions,
- },
- true,
- );
const getEchartsTheme = (options: any) => {
const antdTheme = theme;
const echartsTheme = {
@@ -212,6 +203,9 @@
axisPointer: {
lineStyle: { color: antdTheme.colorPrimary },
label: { color: antdTheme.colorText },
},
+ aria: {
+ enabled: true,
+ },
} as any;
if (options?.xAxis) {
echartsTheme.xAxis = {
@@ -276,7 +270,7 @@
const themedEchartOptions = mergeEchartsThemeOverrides(
baseTheme,
echartOptions,
- globalOverrides,
+ globalOverrides,
chartOverrides,
animationOverride,
);
@@ -288,4 +282,4 @@
chartRef.current?.setOption(themedEchartOptions, {
notMerge,
replaceMerge: notMerge ? undefined : ['series'],
- lazyUpdate: isDashboardRefreshing,
+ lazyUpdate: isDashboardRefreshing,
});
}
}, [didMount, echartOptions, eventHandlers, zrEventHandlers, theme, vizType]);
Code Review Run #a6ac45
Should Bito avoid suggestions like this for future reviews? (Manage Rules)
- Yes, avoid them
|
Thanks for the patience @frassinier, we're still excited for this one. As mentioned, let's put it behind a config flag so admins can opt in while we wait on the User Preferences SIP. Then I think we can get it across the line. I'll see about helping CI along a bit in the meantime. |
|
I'll see if I can help CI along here for now... |
|
Rebased this onto current |
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #28171 +/- ##
==========================================
+ Coverage 57.10% 66.42% +9.31%
==========================================
Files 2857 2727 -130
Lines 161125 150948 -10177
Branches 37060 35654 -1406
==========================================
+ Hits 92014 100263 +8249
+ Misses 68259 48970 -19289
- Partials 852 1715 +863
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Code Review Agent Run #30d26fActionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
|
@frassinier if the diff look OK to you as it is now, I'm happy with the theme-driven approach. My only thing is I think we should default to "off" then anyone can customize their theme to enable it. |
Per review discussion, keep this opt-in via theme overrides until the User Preferences SIP lands, rather than changing default behavior for all deployments.
Code Review Agent Run #1a4da9Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
rusackas
left a comment
rusackas
left a comment
There was a problem hiding this comment.
Default-off change looks right, and it's opt-in via theme override until the User Preferences work lands. CI's green (those cypress/npm failures were the same infra flake going around today, cleared on rerun). Thanks for staying with this one, @frassinier.
rusackas
added
the
merge-if-green
SUMMARY
Increasing accessibility support in Apache/Superset through enhanced a11y features, such as generating aria-label with Echarts, is crucial for ensuring inclusivity and usability for all users, including those with disabilities.
By adhering to WCAG guidelines and making adjustments to colors and patterns as required, we not only enhance the user experience for individuals with disabilities but also align with standards for accessibility, promoting a more inclusive and equitable platform for all users.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION