Added auto-hide timeout for Tooltip component after 1s on touch based devices - #243
Conversation
📝 WalkthroughWalkthroughThe PR implements auto-hide timeout for the Tooltip component on touch devices with 1-second auto-hide behavior, adds client-side pagination to the credits page, and standardizes code formatting from single to double quotes across the entire codebase. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Browser
participant TooltipComponent as Tooltip<br/>Component
participant Timer as Timer<br/>(hideTimeoutRef)
participant DOM as DOM/UI
rect rgba(100, 200, 100, 0.5)
Note over Browser: Touch Device Detection (On Mount)
Browser->>TooltipComponent: useEffect detects navigator.maxTouchPoints
TooltipComponent->>TooltipComponent: Set isTouchDevice state
end
rect rgba(100, 150, 200, 0.5)
Note over User,DOM: Touch Device - Click/Focus Interaction
User->>DOM: Click or Focus on child element
DOM->>TooltipComponent: onClick/onFocus handler triggered
TooltipComponent->>TooltipComponent: Call showTooltip()
TooltipComponent->>TooltipComponent: Set isOpen = true
TooltipComponent->>DOM: Render tooltip (visible)
TooltipComponent->>Timer: Set hideTimeout (1000ms)
end
rect rgba(200, 100, 100, 0.5)
Note over Timer,DOM: Auto-Hide After 1 Second
Timer->>Timer: 1000ms elapsed
TooltipComponent->>TooltipComponent: Set isOpen = false
TooltipComponent->>DOM: Hide tooltip
Timer->>TooltipComponent: Clear hideTimeoutRef
end
rect rgba(150, 150, 100, 0.5)
Note over User,DOM: Desktop Device - Hover/Focus (Unchanged)
User->>DOM: Hover or Focus on element
DOM->>TooltipComponent: Standard ReactTooltip behavior
TooltipComponent->>DOM: Show/hide via hover/focus state
Note over TooltipComponent: isTouchDevice = false<br/>Standard behavior preserved
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@docs/docs/reference/react/components/Tooltip/index.md`:
- Around line 101-103: The implementation notes in the Tooltip docs are
concatenated into run-on lines; split each bullet into its own line/bullet so
each item is separate and readable. Specifically, ensure the lines referencing
`children` wrapper, `useEffect` touch detection, `onClick` handler preservation
(cloning child elements), `isOpen` controlled state and `ReactTooltip`, and the
`appendTo={document.body}` with `positionStrategy="fixed"` are each on their own
bullet/line in the Markdown so they render as separate list items.
In `@docs/docs/reference/react/components/Tooltip/tests.md`:
- Around line 29-31: The test description "auto-hides tooltip after 1 second on
touch devices" is inconsistent with the recommendations: update the docs and/or
the test so they match—either change the description to state it uses fake
timers and show use of vi.useFakeTimers() (and advanceTimersByTime instead of
waiting 1100ms) referencing the test behavior that currently uses
setTimeout/wait 1100ms, or change the recommendations to permit real timers and
explain the 1100ms real-wait behavior (and justify the 7000ms test timeout);
ensure references to vi.useFakeTimers(), advanceTimersByTime, and the existing
setTimeout/wait usage in the test are corrected so documentation and
implementation align.
In `@src/components/Tooltip.jsx`:
- Around line 55-63: The showTooltip function schedules a 1s setTimeout without
storing or clearing its id, causing potential state updates after unmount and
multiple queued timeouts on rapid interactions; fix by storing the timeout id in
a ref (e.g., tooltipTimeoutRef) inside the component, clear any existing timeout
before setting a new one in showTooltip, and also clear the timeout in a
useEffect cleanup (or on component unmount) to prevent calling setIsOpen on an
unmounted component; keep references to isTouchDevice and setIsOpen unchanged
while ensuring tooltipTimeoutRef is used to manage and clear the timer.
In `@src/components/Tooltip.test.jsx`:
- Around line 84-108: The tests in the "Touch device behavior" describe block
mutate navigator.maxTouchPoints via Object.defineProperty without restoring it,
causing cross-test pollution; add a beforeEach that saves the original
navigator.maxTouchPoints (e.g., let originalMaxTouchPoints) and an afterEach
that restores it with Object.defineProperty (configurable/writable true and
value originalMaxTouchPoints) so each test resets navigator; apply the same
beforeEach/afterEach pattern to the "Preserving existing onClick handlers"
describe block as well to ensure isolation.
🧹 Nitpick comments (7)
src/components/Tooltip.jsx (4)
3-3: Formatting inconsistency.The import statement has inconsistent spacing. The pipeline is also flagging Prettier issues.
🔧 Suggested fix
-import {useState,useEffect, useId, cloneElement, isValidElement } from 'react'; +import { useState, useEffect, useId, cloneElement, isValidElement } from 'react';
8-9: Formatting: add semicolons and consistent spacing.State declarations are missing semicolons and have inconsistent indentation compared to the rest of the file.
🔧 Suggested fix
-const [isOpen,setIsOpen] = useState(false) -const [isTouchDevice,setIsTouchDevice] = useState(false) + const [isOpen, setIsOpen] = useState(false); + const [isTouchDevice, setIsTouchDevice] = useState(false);
65-69:handleFocusis redundant.
handleFocusjust delegates toshowTooltip, which already checksisTouchDevice. The extra wrapper and duplicate check are unnecessary.♻️ Simplify by using `showTooltip` directly
-const handleFocus = () =>{ - if(isTouchDevice){ - showTooltip(); - } -}Then update references:
onFocus:(e) => { children.props.onFocus?.(e) - handleFocus(e) + showTooltip() }- onClick = {showTooltip} onFocus={handleFocus} + onClick={showTooltip} onFocus={showTooltip}
85-87: Formatting: spacing around=in JSX attributes.🔧 Suggested fix
- <span data-tooltip-id={tooltipId} data-tooltip-content={content} tabIndex={0} - onClick = {showTooltip} onFocus={handleFocus} - > + <span + data-tooltip-id={tooltipId} + data-tooltip-content={content} + tabIndex={0} + onClick={showTooltip} + onFocus={showTooltip} + >src/components/Tooltip.test.jsx (3)
110-141: Consider using fake timers instead of real timers for deterministic tests.Using
await new Promise(resolve => setTimeout(resolve, 1100))with a 7000ms test timeout makes tests slow and potentially flaky. The documentation recommends usingvi.useFakeTimers()for these scenarios.♻️ Refactor to use fake timers
test('auto-hides tooltip after 1 second on touch devices', async () => { vi.useFakeTimers(); Object.defineProperty(navigator, 'maxTouchPoints', { writable: true, configurable: true, value: 1, }); const user = userEvent.setup({ advanceTimers: vi.advanceTimersByTime }); render( <Tooltip content="Touch tooltip"> <button>Click me</button> </Tooltip> ); const button = screen.getByText('Click me'); await user.click(button); // Tooltip should be visible initially expect(screen.getByText('Touch tooltip')).toBeInTheDocument(); // Advance timers by 1 second await act(() => { vi.advanceTimersByTime(1000); }); // Tooltip should be hidden await waitFor(() => { expect(screen.queryByRole('tooltip')).not.toBeInTheDocument(); }); vi.useRealTimers(); });
138-140: Inconsistent assertion: usenot.toBeInTheDocument()for consistency.Other tests use
not.toBeInTheDocument()for absence checks, but this usestoBeNull(). While functionally similar forqueryBy*, consistency improves readability.🔧 Suggested fix
await waitFor(() => { - expect(screen.queryByRole('tooltip')).toBeNull(); + expect(screen.queryByRole('tooltip')).not.toBeInTheDocument(); });
143-169: Test assertion is weak; consider verifying tooltip does not appear.The test comment acknowledges that "The tooltip might appear from hover side effect of click" but only asserts that the button exists. This doesn't meaningfully verify the intended behavior (click doesn't trigger tooltip on non-touch devices).
♻️ Suggested improvement
await user.click(button); - // The onClick runs but doesn't show tooltip (showTooltip returns early for non-touch) - // Note: The tooltip might appear from hover side effect of click - // What we're really testing is that the onClick handler doesn't crash - expect(button).toBeInTheDocument(); + // On non-touch devices, click alone shouldn't trigger the controlled tooltip state + // The tooltip may still appear via hover/focus from react-tooltip's native behavior, + // but isOpen state should remain false (not programmatically opened) + // Verify no crash and button is still functional + expect(button).toBeInTheDocument(); + + // Optionally: verify that after unhover, tooltip hides normally + await user.unhover(button); + await waitFor(() => { + expect(screen.queryByRole('tooltip')).not.toBeVisible(); + });
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
scripts/handle-changelog.js (1)
62-73: Remove.mdextension from the Docusaurus ID.Line 66 sets
id: ${fileName}wherefileNameincludes the.mdextension. All other Docasaurus documents in the project use IDs without extensions (e.g.,id: contributing,id: coding-style). The ID should be just the slug without the extension:id: ${fileName.replace(/\.md$/, "")}The date format is safe—standard-version produces ISO format dates (YYYY-MM-DD) when generating CHANGELOG.md, which are valid for filenames and match what the changelogSidebarGenerator expects.
src/pages/Editor/index.jsx (1)
205-213: GuardonPointerUpto skip click/color logic onpointercancel.Per the W3C Pointer Events specification,
pointercancelsignals that the interaction was suppressed/aborted by the UA (due to gestures, OS interruption, palm rejection, etc.). It should only trigger cleanup operations, not the same actions aspointerup. Currently,onPointerCancel={onPointerUp}runs the color logic even when the interaction is canceled, which violates pointer event semantics.Guard in
onPointerUpconst onPointerUp = (e) => { const moved = pointerState.current.moved; @@ viewportRef.current?.classList.remove(styles.grabbing); + if (e.type === "pointercancel") { + return; + } + // Only treat as click if user didn't drag if (!moved && isColorMode) { // Find nearest shape inside SVG const svgRoot = innerRef.current?.querySelector("svg");
♻️ Duplicate comments (4)
src/components/Tooltip.jsx (1)
52-61: Memory leak:setTimeoutnot cleaned up on unmount or rapid interactions.This issue was previously flagged. The
setTimeoutreference is not stored, so it cannot be cleared if the component unmounts before the timeout fires, or if the user rapidly taps causing multiple queued timeouts and flickering behavior.src/components/Tooltip.test.jsx (1)
84-227: Restorenavigator.maxTouchPointsbetween tests.
These blocks override a global without cleanup, causing cross-test pollution. Add beforeEach/afterEach to save/restore the original value.Suggested cleanup pattern
+ let originalMaxTouchPoints; + + beforeEach(() => { + originalMaxTouchPoints = navigator.maxTouchPoints; + }); + + afterEach(() => { + Object.defineProperty(navigator, "maxTouchPoints", { + writable: true, + configurable: true, + value: originalMaxTouchPoints, + }); + });Also applies to: 229-355
docs/docs/reference/react/components/Tooltip/tests.md (1)
29-31: Inconsistency between “real timers” description and fake-timer recommendations.Also applies to: 76-79, 91-95
docs/docs/reference/react/components/Tooltip/index.md (1)
101-103: Implementation notes bullets are still run-on; needs line breaks.
🧹 Nitpick comments (7)
docs/docs/writing-documentation/extras/translate-your-site.md (1)
16-17: Unrelated formatting changes should be in a separate PR.This file contains only cosmetic quote-style changes (single → double quotes) in i18n configuration examples, which are unrelated to the PR's stated objective of adding touch-device support to the Tooltip component. Mixing unrelated changes makes the PR harder to review, understand, and potentially revert if needed.
Consider moving these formatting changes to a dedicated style-consistency PR.
Also applies to: 63-63
docs/docs/reference/react/components/GlassSwitch/index.md (2)
76-76: Consider multi-line formatting for documentation readability.This single-line JSX is ~200+ characters, which may be difficult to read in documentation and cause horizontal scrolling. Multi-line formatting would improve clarity for readers.
📝 Suggested formatting
- return <GlassSwitch isOn={notificationsOn} onChange={() => setNotificationsOn(!notificationsOn)} ariaLabel="Toggle notifications" thumbContent={notificationsOn ? <Bell size={16} /> : <BellOff size={16} />} />; + return ( + <GlassSwitch + isOn={notificationsOn} + onChange={() => setNotificationsOn(!notificationsOn)} + ariaLabel="Toggle notifications" + thumbContent={notificationsOn ? <Bell size={16} /> : <BellOff size={16} />} + /> + );
220-222: Same readability suggestion for the Dark Mode Toggle example.The single-line JSX at line 222 has similar readability concerns as the previous example.
📝 Suggested formatting
- return <GlassSwitch isOn={isDark} onChange={toggleTheme} ariaLabel={`Switch to ${isDark ? "light" : "dark"} mode`} thumbContent={isDark ? <Moon size={18} /> : <Sun size={18} />} />; + return ( + <GlassSwitch + isOn={isDark} + onChange={toggleTheme} + ariaLabel={`Switch to ${isDark ? "light" : "dark"} mode`} + thumbContent={isDark ? <Moon size={18} /> : <Sun size={18} />} + /> + );docs/docs/reference/react/components/ThemeSwitch/index.md (1)
142-142: Consider multi-line formatting for the implementation example.Similar to the GlassSwitch documentation, this long single-line JSX could benefit from multi-line formatting for better readability in the documentation context.
📝 Suggested formatting
- return <GlassSwitch isOn={isDark} onChange={toggleTheme} thumbContent={isDark ? <Sun /> : <Moon />} ariaLabel={`switch to ${isDark ? "light" : "dark"} mode`} />; + return ( + <GlassSwitch + isOn={isDark} + onChange={toggleTheme} + thumbContent={isDark ? <Sun /> : <Moon />} + ariaLabel={`switch to ${isDark ? "light" : "dark"} mode`} + /> + );package.json (1)
194-194: Consider aligning prettier's versioning strategy with other devDependencies or document the rationale for exact pinning.The prettier version is pinned exactly to
3.8.1(latest stable), while most other devDependencies use caret ranges (e.g.,^9.39.2for eslint). If exact pinning is intentional for build reproducibility, document this decision; otherwise, consider aligning to^3.8.1for consistency.src/pages/Credits/DependencyCreditsCard.jsx (1)
12-21: Consider adding error handling for the fetch call.If the fetch fails (network error, 404, etc.), the component will silently fail and leave
depsanddevDepsas empty arrays. While this is acceptable fallback behavior for a credits page, you might want to log errors or show a user-friendly message.♻️ Optional: Add basic error handling
useEffect(() => { const url = "https://raw.githubusercontent.com/Ryan-Millard/Img2Num/main/package.json"; fetch(url) .then((res) => res.json()) .then((data) => { setDeps(Object.entries(data.dependencies || {})); setDevDeps(Object.entries(data.devDependencies || {})); - }); + }) + .catch((err) => console.error("Failed to fetch package.json:", err)); }, []);scripts/handle-changelog.js (1)
78-86: Git commit may fail if changelog already exists.If running this script twice for the same release, the
git commiton line 81 will fail because there are no new changes to commit. Consider using--allow-emptyor checking if there are staged changes first.♻️ Proposed fix to handle already-committed scenarios
try { // Stage the changelog folder execSync(`git add ${outPath} ${completeChangelogPath}`, { stdio: "inherit" }); - execSync(`git commit -m "chore(changelog): add ${version} release notes"`, { stdio: "inherit" }); + // Only commit if there are staged changes + try { + execSync("git diff --cached --quiet"); + console.log("[git] No changes to commit."); + } catch { + execSync(`git commit -m "chore(changelog): add ${version} release notes"`, { stdio: "inherit" }); + console.log("[git] docs/changelog added and committed successfully."); + } - - console.log("[git] docs/changelog added and commit amended successfully."); } catch (err) { console.error("[git] Error:", err.message); process.exit(1); }
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@docs/docs/reference/react/components/Tooltip/tests.md`:
- Around line 79-82: In the docs snippet that explains the auto-hide wait,
hyphenate the compound adjective by changing the parenthetical "(1 second
timeout + 100ms buffer)" to "(1-second timeout + 100ms buffer)"; update the text
around the `await new Promise((resolve) => setTimeout(resolve, 1100))` example
so it uses "1-second" as the modifier.
In `@src/components/Tooltip.jsx`:
- Around line 61-71: The showTooltip handler currently calls
e?.stopPropagation?.(), which prevents parent handlers from receiving touch
events; unless tooltip behavior explicitly requires blocking bubbling, remove
that call from showTooltip (or conditionally call it only when a new prop like
blockTouchPropagation is true) so touch clicks bubble normally; update
references in the showTooltip function and ensure hideTimeoutRef and
isTouchDevice logic remain unchanged.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/components/Tooltip.jsx`:
- Line 106: Run Prettier on src/components/Tooltip.jsx to fix the CI style
warning: format the JSX line containing the span element (the element using
data-tooltip-id={tooltipId}, data-tooltip-content={content}, tabIndex={0},
onClick={showTooltip}, onFocus={handleFocus}) so it matches the project's
Prettier rules (or run your repo's formatting script/pre-commit hook) and commit
the updated file.
- Around line 119-121: Remove the unsupported openOnFocus prop from the Tooltip
component: in the JSX block where fallbackPlacements and isOpen are set (near
the existing fallbackPlacements={dynamicPositioning ? ["bottom", "top", "left"]
: []} and isOpen={isTouchDevice ? isOpen : undefined}), delete the openOnFocus
attribute so focus handling falls back to the library's default openEvents; keep
the isOpen conditional for touch devices unchanged so non-touch devices remain
uncontrolled.
♻️ Duplicate comments (1)
src/components/Tooltip.jsx (1)
63-67: Avoid stopping propagation on touch unless explicitly required.
This still blocks parent click handlers on touch devices.
|
@codevory is attempting to deploy a commit to the Ryan Millard's projects Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@package.json`:
- Around line 194-195: Remove the duplicate "prettier" key in devDependencies:
keep only one entry for "prettier" (either the pinned "3.8.1" or the semver
"^3.8.1") and delete the other so the JSON has a single unique key; update
package.json's devDependencies to contain a single "prettier" entry and ensure
the change is intentional for this PR (revert if unrelated to Tooltip
touch-device support).
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/components/Tooltip.jsx`:
- Line 1: This file fails CI due to formatting; run Prettier on the Tooltip
component (e.g., run `prettier --write src/components/Tooltip.jsx`) or apply the
project's Prettier rules to the Tooltip component (Tooltip.jsx) so imports, JSX
and PropTypes usage are formatted correctly; after formatting, re-run lint/CI to
confirm the formatting errors are resolved.
- Line 120: The Tooltip component currently uses a direct reference
appendTo={document.body} which will throw during SSR; update Tooltip.jsx to
avoid accessing document at render by conditionally supplying appendTo (e.g.,
check typeof document !== "undefined" before using document.body) or set append
target after mount (useEffect to set a local state like appendTarget and pass
that to the Tooltip). Ensure the change targets the appendTo prop usage in the
Tooltip component so server renders do not access document.
🧹 Nitpick comments (3)
src/components/Tooltip.jsx (3)
66-70: Unnecessary defensive check —useRefalways returns a stable ref object.The check for
hideTimeoutRefvalidity is defensive code that will never trigger in normal React usage.useRefguarantees a stable, mutable ref object across renders. This adds dead code that obscures the actual logic.♻️ Suggested simplification
const showTooltip = () => { if (!isTouchDevice) return; - - // Bail if ref is not a valid ref object (defensive in hot-reload scenarios) - if (!hideTimeoutRef || typeof hideTimeoutRef !== "object") { - return; - } - // Clear any existing timeout if (hideTimeoutRef.current) { clearTimeout(hideTimeoutRef.current);
77-81: Functional update does not prevent re-renders as intended.React's state bailout optimization only works with direct value comparison, not functional updates. Calling
setIsOpen((prev) => prev)still schedules a render. To prevent unnecessary work, checkisOpenbefore calling the setter.♻️ Suggested fix
- // If already open, don't re-open (prevents unnecessary re-render) - setIsOpen((prev) => { - if (prev) return prev; // Already open, don't update - return true; - }); + setIsOpen(true);Since
isOpenis only used for touch devices andshowTooltipalready bails early if!isTouchDevice, the simplesetIsOpen(true)is sufficient. React will batch and dedupe if alreadytrue.
109-111: Addrole="button"for accessibility on the interactive span.The fallback
<span>hasonClickandtabIndex={0}making it interactive, but lacks aroleattribute. Screen readers won't announce it as an actionable element without an explicit role.♿ Suggested fix
- <span data-tooltip-id={tooltipId} data-tooltip-content={content} tabIndex={0} onClick={showTooltip} onFocus={handleFocus}> + <span data-tooltip-id={tooltipId} data-tooltip-content={content} tabIndex={0} role="button" onClick={showTooltip} onFocus={handleFocus}>
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pages/Credits/DependencyCreditsCard.jsx (1)
39-40: Replace deprecatedcacheTimewithgcTimefor React Query v5 compatibility.In TanStack Query v5,
cacheTimewas renamed togcTime(garbage collection time). This option controls how long an inactive query remains cached before being garbage-collected.Suggested fix
staleTime: ONE_WEEK, - cacheTime: ONE_WEEK, + gcTime: ONE_WEEK,
🧹 Nitpick comments (3)
src/pages/Credits/DependencyCreditsCard.jsx (1)
22-27: Consider adding error handling for the fetch call.The fetch call lacks error handling. If the GitHub raw URL is unreachable or returns an error, the component will silently fail without user feedback.
Suggested improvement
fetch(url) .then((res) => res.json()) .then((data) => { setDeps(Object.entries(data.dependencies || {})); setDevDeps(Object.entries(data.devDependencies || {})); - }); + }) + .catch((err) => { + console.error("Failed to fetch package.json:", err); + });src/pages/Credits/ContributorsCreditsCard.jsx (1)
23-52: Consider rendering only the active page directly instead of mapping all tables.The current approach maps over all table chunks but conditionally renders only when
i === page. This creates unnecessary iterations. You can rendertables[page]directly for cleaner and slightly more efficient code.Suggested simplification
<div className={styles.contributorsGrid}> - {tables.map( - (group, i) => - i === page && ( - <table key={i}> - <tbody> - {group.map((c) => ( - <tr key={c.id}> + {tables[page] && ( + <table> + <tbody> + {tables[page].map((c) => ( + <tr key={c.id}> <td> <Tooltip content={`Open ${c.login}'s GitHub profile`}> <a href={c.html_url} target="_blank" rel="noopener noreferrer"> <FallbackImage src={c.avatar_url} fallback={<User color="var(--color-text-light)" />} alt={c.login} width="28" height="28" className={styles.avatar} /> </a> </Tooltip> </td> <td> <Tooltip content={`Visit ${c.login}'s GitHub profile`}> <a href={c.html_url} target="_blank" rel="noopener noreferrer"> {c.login} </a> </Tooltip> </td> <td> {c.contributions} {c.contributions === 1 ? "commit" : "commits"} </td> </tr> ))} </tbody> </table> - ), - )} + )} </div>src/components/Tooltip.jsx (1)
21-60: Add legacyMediaQueryListlistener fallback for older browsers.
Lines 50–57 only attachaddEventListener/removeEventListener. Older Safari (pre-14) lacksaddEventListenersupport onMediaQueryListand requiresaddListener/removeListenerinstead.♻️ Suggested fallback
- if (mediaQuery.addEventListener) { - mediaQuery.addEventListener("change", handleChange); - } + if (mediaQuery.addEventListener) { + mediaQuery.addEventListener("change", handleChange); + } else if (mediaQuery.addListener) { + mediaQuery.addListener(handleChange); + } ... - if (mediaQuery?.removeEventListener) { - mediaQuery.removeEventListener("change", handleChange); - } + if (mediaQuery?.removeEventListener) { + mediaQuery.removeEventListener("change", handleChange); + } else if (mediaQuery?.removeListener) { + mediaQuery.removeListener(handleChange); + }
|
@codevory, don't worry about SSR suggestions from @CodeRabbit - they're a waste of time because we don't use SSR and don't plan on using it at all. |
|
Tip For best results, initiate chat on the files or code changes.
✏️ Learnings added
|
2 participants
What was changed & why
Addressing the issue #168 I have added auto hide timeout of 1 sec. to Tooltip componet for touch devices like Mobile/Tablet etc.
Fixed someother bugs like The onClick setup in the cloneElement call replaces any existing click handlers on the tooltip's children mentioned in PR #173 , The existing progress made in PR #173 made it easy for me to solve it quickly Thanks to guy who had solved it almost & to maintainer @Ryan-Millard as well .
Fixes: #168
Changes
✨ What's Added
Touch Device Support for Tooltips:
Touchdevice detection using multiple methods ([navigator.maxTouchPoints] , media queries, ontouchstart)
Click-to-reveal - Tapping elements shows tooltips on touch devices
Focus-to-reveal - Keyboard navigation (Tab) triggers tooltips on touch devices
Auto-hide after 1 second - Tooltips automatically dismiss since hover-out isn't available
Dynamic detection - Listens for input changes (e.g., mouse plugged into tablet)
Event handler preservation - Existing [onClick] and [onFocus] handlers are merged and called before tooltip logic
Documentation
Updated component documentation with Touch Device Support section
Added Implementation notes explaining the new behavior
Comprehensive test documentation with 15 test case explanations
Testing recommendations for timer mocking and navigator property mocking
Testing & Verification
5 comprehensive tests covering
Additional Resources
Technical Implementation
Uses React state ( isOpen, isTouchDevice ) for controlled tooltip visibility.
useEffect hook for client-side device detection with cleanup.
Event handler merging pattern preserves existing functionality
isOpen prop on ReactTooltip enables manual control for touch devices
Img2Num.Image-to-Color-by-Number.Templates.-.Google.Chrome.2026-01-24.21-05-01.mp4
Summary by CodeRabbit
New Features
Documentation
Style
✏️ Tip: You can customize this high-level summary in your review settings.