Upgrade mantine and catch fullscreen error#2092
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
The toggle function from Mantine's useFullscreen hook is async and can
reject when requestFullscreen() fails (eg. due to browser security
policy, extensions blocking fullscreen, or similar restrictions).
Using it directly as onClick={toggle} left the rejected Promise
unhandled, causing a browser 'Uncaught (in promise) DOMException' error.
The fix wraps the toggle call in a try/catch inside a dedicated
handleToggleFullscreen callback. The navigation menu is also closed
before the fullscreen request is made, which avoids any potential
focus-management interference from the open dialog and gives better UX.
https://claude.ai/code/session_01HQ9UzKrKDR3h9JuSUYzr4b
Mantine's useFullscreen was only used in one place, and its toggle() returns an async Promise that leaks into callers. The new useFullscreen hook in common/hooks: - Exposes isSupported so the call site has a single source of truth (removes the separate supportsFullscreen constant from externals.ts) - toggle() is a plain void callback — errors from requestFullscreen / exitFullscreen are caught internally, so it is always safe to use directly as an onClick handler without any wrapping - toggle() is a no-op when isSupported is false - Targets document.documentElement only (the ref-based API from Mantine was never used in this project) - No vendor-prefix shims — all supported browsers implement the standard Fullscreen API https://claude.ai/code/session_01HQ9UzKrKDR3h9JuSUYzr4b
| @@ -0,0 +1,21 @@ | |||
| import { useFullscreenDocument } from '@mantine/hooks'; | |||
There was a problem hiding this comment.
this is a basic operation which I dont think warrants too many abstraction layers. I think we are better off making our own
| data-disabled={disable} | ||
| onKeyDown={(event) => { | ||
| if (isKeyEnter(event)) { | ||
| if (!!disable && isKeyEnter(event)) { |
There was a problem hiding this comment.
this seems incorrect or at least confusing.
!! is usually used as a boolean casting operator, which we often prefer to use the boolean constructor instead to make the code explicit
but here I think you are trying to negate the boolean like !disable && isKeyEnter?
ie: if the element is not disabled, and user pressed enter, we do the thing
3 participants
No description provided.