Make WordPress Core

Opened 9 years ago

Closed 9 years ago

#30348 closed defect (bug) (fixed)

Arrow key navigation in Media Grid skips ids

Reported by: umeshsingla's profile UmeshSingla Owned by: johnbillion's profile johnbillion
Milestone: 4.1 Priority: normal
Severity: normal Version: 4.0
Component: Media Keywords: has-patch commit
Focuses: accessibility, javascript Cc:

Description (last modified by ocean90)

In grid view, on opening the attachment (Attachment Details screen ),, While using arrow keys for navigation, initially the next image loaded is proper,, but after that it skips a id each time

To replicate:

Step - 1:

Upload some images and switch to grid view in WordPress

Step - 2:

Click over a image from grid view, which opens attachment details screen for that media. Use the right arrow key for navigation.

Step -3

On pressing right arrow key, firs time the dialog loads the proepr image, but when you press the arrow back it starts skipping the image.

Attachments (2)

30348.diff (373 bytes) - added by UmeshSingla 9 years ago.
fixes the bug in media-grid.js
30348.2.diff (877 bytes) - added by adamsilverstein 9 years ago.
regenerate patch from src, clean up inline doc description

Download all attachments as: .zip

Change History (8)

9 years ago

fixes the bug in media-grid.js

#1 @UmeshSingla
9 years ago

On opening modal, this code already handles the keyDown event:

this.modal.on( 'open', function () {
  $( 'body' ).on( '', _.bind( self.keyEvent, self ) );
} );

And code at, registers the keyDown event again, which causes the nextMediaItem to call twice and thus skip ids.

#2 @5um17
9 years ago

  • Keywords has-patch added
  • Version changed from trunk to 4.0

9 years ago

regenerate patch from src, clean up inline doc description

#3 @adamsilverstein
9 years ago

  • Keywords commit added
  • Milestone changed from Awaiting Review to 4.1

@UmeshSingla - Wow, thanks for catching that! Having spent a good bit of time working on and testing keyboard navigation in the media grid, I'm embarrassed that we missed that!

I tested your patch and verified the bug exists before applying and is fixed after applying. Please note that convention is to generate diffs from the /trunk folder, makes it a bit easier to apply them :) I regenerated the patch from there and also cleaned up the language describing the keyEvent function.

Thanks again, great work!

#4 @ocean90
9 years ago

  • Description modified (diff)

#5 @UmeshSingla
9 years ago

@adamsilverstein, well, that's one hell of a appreciation I got ever, thanks :)

Last edited 9 years ago by UmeshSingla (previous) (diff)

#6 @johnbillion
9 years ago

  • Owner set to johnbillion
  • Resolution set to fixed
  • Status changed from new to closed

In 30377:

Avoid re-registering the keydown event controller in the media grid so arrow controls don't skip media items.

Fixes #30348
Props UmeshSingla

Note: See TracTickets for help on using tickets.