fix: trap keyboard focus within onboarding dialog - #885
fix: trap keyboard focus within onboarding dialog#885karthikeya20012007 wants to merge 5 commits into
Conversation
|
@karthikeya20012007 is attempting to deploy a commit to the magic-peach1's projects Team on Vercel. A member of the Team first needs to authorize it. |
✅ PR Format Check Passed — @karthikeya20012007Basic format checks passed. A maintainer will review your code changes. This does not mean the PR is approved — it just means the format is correct. |
👋 Thanks for your PR, @karthikeya20012007!Welcome to Reframe — a browser-based video editor built for everyone 🎬
What happens next
Quick checklist
Useful links
Happy coding! 🎉 |
|
Hey @karthikeya20012007! Thanks for fixing keyboard focus trapping in the onboarding dialog — this is an important accessibility fix. This PR currently has merge conflicts with git fetch origin
git rebase origin/main
# resolve any conflicts
git push --force-with-lease origin <your-branch>Once rebased and CI passes, I'll review and merge this. |
|
Hey @karthikeya20012007! This PR had a merge conflict with `main` due to a concurrent refactor of `OnboardingTour.tsx` — I've resolved it automatically and pushed the result. Here's what was merged:
CI has been re-triggered. The focus trap implementation looks correct — `focus-trap-react` is already in `package.json` and this is a solid accessibility improvement. |
|
Thanks for resolving the conflicts and integrating the refactor changes. I appreciate the review and the detailed explanation! |
|
@karthikeya20012007 This PR has merge conflicts with Please rebase your branch on the latest git fetch upstream
git rebase upstream/main
# For each conflict in src/components/OnboardingTour.tsx:
# - Keep your FocusTrap wrapper in the return statement
# - Keep main's tryMeasure retry logic (the retryCount/retryTimer approach)
# - Keep main's remeasure with requestAnimationFrame + scroll listener
# Then:
git add src/components/OnboardingTour.tsx
git rebase --continue
git push --force-with-lease origin fix/onboarding-focus-trapOnce rebased and CI passes, this will be merged quickly! |
|
Hi @magic-peach, just following up — I synced the branch with the latest upstream changes and resolved the remaining merge conflicts locally. The branch should now be up to date on my side. Please let me know if any additional changes are needed from me. |
|
Hi @magic-peach, just following up — I synced the branch with the latest upstream changes and resolved the remaining merge conflicts locally. The branch should now be up to date on my side. |
|
Hi @karthikeya20012007 — good news and a small ask. Your PR passed review in our backlog cleanup and was queued to merge. We merged 38 PRs today, and yours now conflicts with To land it: git fetch origin
git rebase origin/main
# resolve conflicts
git push --force-with-leasePing me here once it's green and I'll merge it straight away — it's already approved on our side, so it won't go back into the queue. Thanks for your patience with how long this sat 🙏 |
…agic-peach#892) When speed is zero or negative, the while loop dividing remaining by 0.5 never converges: 0 / 0.5 == 0, causing an infinite loop that blocks the browser thread entirely. Return an empty filter string immediately for any speed <= 0, which is consistent with the existing behavior of omitting atempo filters when no adjustment is needed. Closes magic-peach#862
…N dimensions (magic-peach#894) NaN comparisons (NaN < 16, NaN > 7680) always evaluate to false, so a customWidth or customHeight that is NaN passes all boundary checks and validateRecipe returns null (no error). The NaN then propagates to the FFmpeg scale filter as scale=NaN:NaN, crashing the export immediately. Two fixes applied: 1. validateRecipe: prefix each custom-dimension check with Number.isNaN() so a NaN value is caught as a validation error before any comparison. 2. Legacy localStorage parser: replace bare nullish-coalescing on parsed.customWidth/customHeight with a sanitizeDimension helper that calls Number.isFinite() and enforces the [16, 7680] range. Any corrupted, non-numeric, or out-of-range stored value falls back to the previous safe default instead of being merged into state as NaN. Closes magic-peach#864
2388541 to
79c7bfd
Compare
Description
Fixes an accessibility issue where keyboard focus could escape the onboarding dialog while the tour was active.
Added
FocusTrapto keep keyboard navigation contained within the onboarding modal and preserved existing dismissal/navigation behavior.Related Issue
Closes #878
Type of Contribution
Participant Info
Screen Recording
Recording / Loom link:
Checklist
bun run lintpasses (no ESLint errors)bunx tsc --noEmitpasses (no TypeScript errors)aria-label/ accessible namesconsole.logstatements left in