fix: prevent parent scroll on keyboard navigation in preview - #526
fix: prevent parent scroll on keyboard navigation in preview#526waterWang wants to merge 1 commit into
Conversation
When the Image preview is open and the user presses LEFT/RIGHT arrow keys to switch images, the browser's default scroll behavior was not prevented. This caused the parent element to scroll instead of the Image preview navigating between images. The fix adds event.preventDefault() when handling LEFT and RIGHT key events in the Preview component, preventing the browser's default scroll behavior. Closes ant-design/ant-design#56290
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
@waterWang is attempting to deploy a commit to the afc163's projects Team on Vercel. A member of the Team first needs to authorize it. |
Walkthrough้ข่ง็ป็ๅทฆๅณๆนๅ้ฎๅค็ๆฐๅข Changes้ข่ง้ฎ็ๅฏผ่ช
Estimated code review effort: 1 (็ฎๅ) | ~5 minutes Suggested reviewers: Poem
๐ฅ Pre-merge checks | โ 5โ Passed checks (5 passed)
โจ Finishing Touches ๐ก 1๐ ๏ธ Fix failing CI checks ๐ก
๐งช 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 |
There was a problem hiding this comment.
๐งน Nitpick comments (1)
src/Preview/index.tsx (1)
351-354: ๐ฏ Functional Correctness | ๐ต Trivial | โก Quick winไธบๆนๅ้ฎ้ป่ฎค่กไธบๅขๅ ๅๅฝๆญ่จใ
็ฐๆ
tests/previewGroup.test.tsxๅช้ช่ฏๅพ็ๅๆข๏ผๆฒกๆ้ช่ฏevent.defaultPreventedใ่ฏทไธบ LEFT ๅ RIGHT ไบไปถๅขๅ ๆญ่จ๏ผ็กฎไฟๅ็ปญๆนๅจไธไผ็งป้คevent.preventDefault()ใ๐ค Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Preview/index.tsx` around lines 351 - 354, ๅจ tests/previewGroup.test.tsx ็ๆนๅ้ฎไบไปถๆต่ฏไธญ๏ผไธบ LEFT ๅ RIGHT ไธค็งๆ้ฎๅๅซๅขๅ event.defaultPrevented ๆญ่จ๏ผ็กฎไฟ่งฆๅๅพ็ๅๆขๆถไป่ฐ็จ event.preventDefault()๏ผไฟ็็ฐๆๅพ็ๅๆขๆญ่จไธๅใ
๐ค Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/Preview/index.tsx`:
- Around line 351-354: ๅจ tests/previewGroup.test.tsx ็ๆนๅ้ฎไบไปถๆต่ฏไธญ๏ผไธบ LEFT ๅ RIGHT
ไธค็งๆ้ฎๅๅซๅขๅ event.defaultPrevented ๆญ่จ๏ผ็กฎไฟ่งฆๅๅพ็ๅๆขๆถไป่ฐ็จ
event.preventDefault()๏ผไฟ็็ฐๆๅพ็ๅๆขๆญ่จไธๅใ
โน๏ธ Review info
โ๏ธ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d36a4d48-b6b1-4645-ba05-12ec05679cfd
๐ Files selected for processing (1)
src/Preview/index.tsx
nrps9909
left a comment
There was a problem hiding this comment.
Reviewed exact head 8ee1abcd9d73b24a019d962933e8fdcce8997f5e.
The change is correctly scoped to an open group preview: LEFT/RIGHT still perform the existing image navigation and now cancel the browser's default scrolling behavior after that navigation. Single-image/non-group previews do not enter this branch, and unrelated keys remain uncancelled. I also checked the first/last-image boundary behavior; cancelling LEFT/RIGHT while the modal owns keyboard navigation is consistent even when there is no further image in that direction.
Validation performed on this exact head:
npm test -- tests/previewGroup.test.tsx --runInBandโ 16/16 passednpm test -- --runInBandโ 80/80 passed, 1/1 snapshot- independent temporary probe โ before opening, RIGHT was not cancelled; in an open group preview, RIGHT and LEFT were cancelled while navigating 1/2 โ 2/2 โ 1/2; UP remained uncancelled
npm run tscโ passednpm run lint -- --no-fixโ 0 errors (12 pre-existing warnings)npx prettier --check src/Preview/index.tsxโ passednpm run buildโ passedgit diff --check ca5754037261b89a60681bf85c056d6533adee8d...8ee1abcd9d73b24a019d962933e8fdcce8997f5eโ passed
The reported Vercel failure is an external deployment-authorization check, not a code failure. I found no blocking issue in the change.
Disclosure: Codex assisted with source tracing, test execution, and drafting this review; I independently verified the behavior and conclusion against the exact PR head.
Codecov Reportโ
All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #526 +/- ##
=======================================
Coverage 99.26% 99.26%
=======================================
Files 17 17
Lines 543 545 +2
Branches 166 167 +1
=======================================
+ Hits 539 541 +2
Misses 4 4 โ View full report in Codecov by Harness. ๐ New features to boost your workflow:
|
Description
When the Image preview is open and the user presses LEFT/RIGHT arrow keys to switch images, the browser's default scroll behavior was not prevented. This caused the parent element to scroll instead of the Image preview navigating between images.
Root Cause
The
onKeyDownhandler inPreview/index.tsxhandles LEFT/RIGHT arrow keys to navigate between images, but does not callevent.preventDefault(). The browser's default behavior for arrow keys is to scroll the scrollable parent element.Fix
Added
event.preventDefault()when handling LEFT and RIGHT key events in the Preview component, preventing the browser's default scroll behavior.Related Issue
Closes ant-design/ant-design#56290 โ Image ้ฎ็ๅๆขๅพ็๏ผๅค้จๅ ็ด ไผ่ท็ๆปๅจ
Summary by CodeRabbit