-
Notifications
You must be signed in to change notification settings - Fork 3.2k
fix skip tooltip rendering when element is not focused #61708
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix skip tooltip rendering when element is not focused #61708
Conversation
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2025-05-10.at.17.34.13.mp4Android: mWeb ChromeScreen.Recording.2025-05-10.at.17.28.58.mp4iOS: HybridAppScreen.Recording.2025-05-10.at.17.26.18.mp4iOS: mWeb SafariScreen.Recording.2025-05-10.at.17.27.48.mp4MacOS: Chrome / SafariScreen.Recording.2025-05-10.at.17.13.44.mp4MacOS: DesktopScreen.Recording.2025-05-10.at.17.21.24.mp4 |
…ry-tags-popup-persists-flashes
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
// Skip the tooltip and return the children if the device does not support hovering | ||
if (!hasHoverSupport) { | ||
// Skip the tooltip and return the children if the device does not support hovering or if navigation does not focus. | ||
if (!hasHoverSupport || !isFocused) { | ||
return children; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hasHoverSupport is a function, it should be invoked here. Adding isFocused
causing the tooltip to not show on the LHN.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just FYI, we had a long discussion before about this Safari-only issue here and decided to do nothing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the reference. It looks like we’re missing a test case on LHN @huult could you please take a look on this one
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks, I'm checking
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bernhardoj thank you for reporting this crash. Could you share the steps to reproduce it? Many thanks.
🚀 Deployed to staging by https://github.com/JS00001 in version: 9.1.45-0 🚀
|
🚀 Deployed to staging by https://github.com/JS00001 in version: 9.1.45-0 🚀
|
🚀 Deployed to production by https://github.com/francoisl in version: 9.1.45-21 🚀
|
Details
Fixed Issues
$ #61119
PROPOSAL: #61119 (comment)
Tests
Same QA step
Offline tests
QA Steps
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)myBool && <MyComponent />
.src/languages/*
files and using the translation methodSTYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)Design
label and/or tagged@Expensify/design
so the design team can review the changes.ScrollView
component to make it scrollable when more elements are added to the page.main
branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTest
steps.Screenshots/Videos
Android: Native
Screen.Recording.2025-05-08.at.22.06.53.mp4
Android: mWeb Chrome
Screen.Recording.2025-05-08.at.22.02.33.mp4
iOS: Native
Screen.Recording.2025-05-08.at.22.09.56.mp4
iOS: mWeb Safari
Screen.Recording.2025-05-08.at.22.08.50.mp4
MacOS: Chrome / Safari
Screen.Recording.2025-05-08.at.22.00.48.mp4
MacOS: Desktop
Screen.Recording.2025-05-08.at.21.59.54.mp4