sadpandajoe commented on code in PR #42910:
URL: https://github.com/apache/superset/pull/42910#discussion_r4303319937
##########
superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberPeriodOverPeriod/controlPanel.ts:
##########
@@ -100,21 +125,24 @@ const config: ControlPanelConfig = {
],
[
{
- name: 'comparison_color_scheme',
+ name: 'increase_color',
config: {
- type: 'SelectControl',
- label: t('color scheme for comparison'),
- default: ColorSchemeEnum.Green,
- renderTrigger: true,
- choices: [
- [ColorSchemeEnum.Green, 'Green for increase, red for
decrease'],
- [ColorSchemeEnum.Red, 'Red for increase, green for decrease'],
- ],
- visibility: ({ controls }) =>
- controls?.comparison_color_enabled?.value === true,
+ ...comparisonColorControlConfig,
+ label: t('Color for increase'),
Review Comment:
The old `SelectControl` gave screen-reader users a name from its label, but
`ColorPickerControl` only sets `aria-label` from the `ariaLabel` prop and these
two pickers don't pass one (the Gauge and Bullet controls do). Assistive tech
will announce the color value rather than 'Color for increase' / 'Color for
decrease'. Could `ariaLabel` be added to the shared config or per control?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
