rusackas commented on code in PR #42078:
URL: https://github.com/apache/superset/pull/42078#discussion_r3652261013
##########
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControlShared.tsx:
##########
@@ -236,6 +236,10 @@ export const DescriptionToolTip = ({
whiteSpace: 'normal',
}}
>
+ {/* Deliberate role="button" on this tooltip-trigger icon (no click
+ handler) — covered by existing test expectations; not a fit for
+ the linter's suggested <button> tag. */}
Review Comment:
Fair, DescriptionToolTip doesn't actually have its own test for this.
Reworded the comment to point at the sibling DeckglLayerVisibilityTooltip
pattern instead of claiming coverage it doesn't have.
##########
superset-frontend/src/explore/components/controls/VizTypeControl/VizTypeGallery.tsx:
##########
@@ -344,29 +349,20 @@ const Thumbnail: FC<ThumbnailProps> = ({
const { key, value: type } = entry;
const isSelected = selectedViz === entry.key;
- const handleKeyDown = (event: React.KeyboardEvent) => {
- if (event.key === 'Enter' || event.key === ' ') {
- event.preventDefault();
- setSelectedViz(key);
- }
- };
-
const handleFocus = () => {
// Auto-select chart when tabbed to
setSelectedViz(key);
};
return (
- <div
- role="button"
+ <button
+ type="button"
// using css instead of a styled component to preserve
// the data-test attribute
css={thumbnailContainerCss(theme)}
- tabIndex={0}
className={isSelected ? 'selected' : ''}
onClick={() => setSelectedViz(key)}
onDoubleClick={onDoubleClick}
- onKeyDown={handleKeyDown}
onFocus={handleFocus}
data-test="viztype-selector-container"
Review Comment:
Good catch, added aria-pressed={isSelected} to the thumbnail button.
##########
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/UrlFilters/VerticalCollapse.tsx:
##########
@@ -104,24 +110,13 @@ const UrlFiltersVerticalCollapse = (props: {
return (
<div css={sectionContainerStyle}>
- <div
- css={sectionHeaderStyle}
- onClick={toggleSection}
- onKeyDown={e => {
- if (e.key === 'Enter' || e.key === ' ') {
- e.preventDefault();
- toggleSection();
- }
- }}
- role="button"
- tabIndex={0}
- >
+ <button type="button" css={sectionHeaderStyle} onClick={toggleSection}>
Review Comment:
Added aria-expanded on both section headers, tied to sectionsOpen.
--
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]
