rusackas commented on code in PR #42078:
URL: https://github.com/apache/superset/pull/42078#discussion_r3588318457
##########
superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopoverTitle.tsx:
##########
@@ -113,24 +113,30 @@ const AdhocMetricEditPopoverTitle:
FC<AdhocMetricEditPopoverTitleProps> = ({
return (
<Tooltip placement="top" title={t('Click to edit label')}>
- <span
+ <button
+ type="button"
+ css={css`
+ appearance: none;
+ border: none;
+ background: none;
+ padding: 0;
+ font: inherit;
Review Comment:
Fixed, added `cursor: pointer` to the button.
##########
superset-frontend/src/features/charts/ChartCard.tsx:
##########
@@ -103,9 +115,9 @@ export default function ChartCard({
menuItems.push({
key: 'export',
label: (
- <div
- role="button"
- tabIndex={0}
+ <button
+ type="button"
+ css={menuItemButtonCss}
Review Comment:
Added `data-test="chart-list-export-option"` for parity with edit/delete.
##########
superset-frontend/packages/superset-ui-core/src/components/ActionButton/index.tsx:
##########
@@ -35,14 +39,26 @@ export const ActionButton = ({
placement,
icon,
onClick,
+ className,
+ disabled = false,
+ dataTest,
}: ActionProps) => {
const theme = useTheme();
const actionButton = (
- <span
- role="button"
- tabIndex={0}
+ <button
+ type="button"
+ aria-disabled={disabled}
Review Comment:
This uses `aria-disabled` on purpose so the button stays focusable, that's
the recommended ARIA pattern for disabled controls you still want screen
readers to perceive. Adding `tabIndex={-1}` would undo that.
##########
superset-frontend/packages/superset-ui-core/src/components/ActionButton/index.tsx:
##########
@@ -52,12 +68,12 @@ export const ActionButton = ({
}
}
Review Comment:
Fair, added `color`/`cursor: not-allowed` styling for the `.disabled` state.
--
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]
