rusackas commented on code in PR #42078:
URL: https://github.com/apache/superset/pull/42078#discussion_r3651453513
##########
superset-frontend/packages/superset-ui-core/src/components/ActionButton/index.tsx:
##########
@@ -52,14 +67,17 @@ export const ActionButton = ({
fill: ${theme.colorPrimary};
}
}
+ &.disabled {
+ color: ${theme.colorTextDisabled};
+ cursor: not-allowed;
+ }
Review Comment:
Good catch, scoped the hover to `&:not(.disabled)` so disabled icons no
longer highlight.
##########
superset-frontend/src/explore/components/controls/ColumnConfigControl/ColumnConfigControl.tsx:
##########
@@ -167,10 +166,15 @@ export default function ColumnConfigControl<T extends
ColumnConfig>({
/>
))}
{needShowMoreButton && (
- <div
- role="button"
+ <button
+ type="button"
tabIndex={-1}
css={{
Review Comment:
Right, dropped the `tabIndex={-1}` so the toggle is back in the keyboard tab
order now that it's a real `<button>`.
##########
superset-frontend/src/components/CopyToClipboard/CopyToClipboard.stories.tsx:
##########
@@ -31,7 +31,7 @@ export const InteractiveCopyToClipboard = ({ copyNode,
...rest }: any) => {
if (copyNode === 'Icon') {
node = <Icons.CopyOutlined />;
} else if (copyNode === 'Text') {
- node = <span role="button">Copy</span>;
+ node = <button type="button">Copy</button>;
}
Review Comment:
`CopyToClipboard` clones a valid element instead of wrapping it, so a
`<button>` copyNode gets the handlers cloned onto it rather than nested inside
another button. No nesting here.
--
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]
