fix(explore): prevent duplicate Date Range tooltips - #43425
fix(explore): prevent duplicate Date Range tooltips#43425ayush-singh-0601 wants to merge 1 commit into
Conversation
dosubot
Bot
added
change:frontend
Code Review Agent Run #fec59bActionable 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 |
![codeant-ai-for-open-source[bot]](https://avatars.githubusercontent.com/u/151810221?s=60&v=4){kind=small}
| <Icons.InfoCircleOutlined | ||
| css={iconStyles} | ||
| onClick={tooltipOnClick} | ||
| aria-label={t('Show info tooltip')} | ||
| /> |
There was a problem hiding this comment.
Suggestion: The icon is given an accessible name but remains a non-focusable SVG inside a plain span; keyboard and assistive-technology users cannot reach or activate the existing tooltipOnClick action. Make the icon wrapper keyboard-focusable with appropriate button semantics and keyboard activation, or remove the action-oriented aria-label if the icon is intentionally hover-only. [api mismatch]
Severity Level: Major ⚠️
- ⚠️ Keyboard users cannot reach described Explore controls' info actions.
- ⚠️ Screen readers announce an unavailable button action.
- ⚠️ `tooltipOnClick` remains mouse-only for control headers.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** superset-frontend/src/explore/components/ControlHeader.tsx
**Line:** 113:117
**Comment:**
*Api Mismatch: The icon is given an accessible name but remains a non-focusable SVG inside a plain span; keyboard and assistive-technology users cannot reach or activate the existing `tooltipOnClick` action. Make the icon wrapper keyboard-focusable with appropriate button semantics and keyboard activation, or remove the action-oriented `aria-label` if the icon is intentionally hover-only.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
|
The flagged issue is correct. The Here is a concise fix using a <button
type="button"
data-test={`${name}-description-icon`}
onMouseEnter={() => onDescriptionHoverChange?.(true)}
onMouseLeave={() => onDescriptionHoverChange?.(false)}
onClick={tooltipOnClick}
css={css`
background: none;
border: none;
padding: 0;
cursor: pointer;
`}
aria-label={t('Show info tooltip')}
>
<Tooltip
id="description-tooltip"
title={description}
placement="top"
mouseLeaveDelay={0}
>
<Icons.InfoCircleOutlined css={iconStyles} />
</Tooltip>{' '}
</button>Would you like me to check the rest of the comments on this PR and implement fixes for them as well? superset-frontend/src/explore/components/ControlHeader.tsx |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #43425 +/- ##
==========================================
+ Coverage 78.85% 78.86% +0.01%
==========================================
Files 2876 2876
Lines 164581 164589 +8
Branches 38011 38014 +3
==========================================
+ Hits 129786 129809 +23
+ Misses 32348 32333 -15
Partials 2447 2447
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:
|
1 participant
SUMMARY
Hovering the Date Range info icon in Explore currently opens two tooltips at once: the control description and the resolved time-range tooltip on the field.
The info icon lives in
ControlHeaderand is absolutely positioned next to the label. The Date Range value is wrapped in its own tooltip inDateFilterLabel. Those two hover targets can overlap, so both tooltips fire together.This keeps both tooltips. The field tooltip is suppressed only while the info icon is hovered, the icon stacking is isolated from the field, and the field tooltip closes immediately on mouse leave. Hovering the Date Range field still shows the time-range tooltip.
Fixes #43386
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Before — hovering the info icon shows two tooltips:
After — hovering the info icon shows only the description tooltip:
After — hovering the Date Range field still shows the field tooltip:
TESTING INSTRUCTIONS
docker compose up --buildor your usual setup).iicon next to the Date Range label.Unit coverage is in
DateFilterLabel.test.tsx: it hovers the field, then the info icon, and asserts only one tooltip is shown.ADDITIONAL INFORMATION