[webkit-reviews] review granted: [Bug 178800] Web Inspector: Canvas Tab: canvases overview should support navigation via keyboard : [Attachment 324974] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Oct 26 10:29:32 PDT 2017


Brian Burg <bburg at apple.com> has granted Devin Rousso
<webkit at devinrousso.com>'s request for review:
Bug 178800: Web Inspector: Canvas Tab: canvases overview should support
navigation via keyboard
https://bugs.webkit.org/show_bug.cgi?id=178800

Attachment 324974: Patch

https://bugs.webkit.org/attachment.cgi?id=324974&action=review




--- Comment #3 from Brian Burg <bburg at apple.com> ---
Comment on attachment 324974
  --> https://bugs.webkit.org/attachment.cgi?id=324974
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=324974&action=review

r=me 

General comments:

Whew, that's way hairier than I thought.
- What do you think about pushing these shortcuts down into
CollectionContentView? I guess that might be kind of messy if collection
elements are not sized uniformly, but it should still be possible to observe
visual rows and columns regardless of the flex direction if you creep forward
and backward enough from the current selection to find the next/prev row. (We
can shelve this for later, just a thought.)
- RTL would change visual order but not the logical order. Can you make sure
that these shortcuts work as intended?

> Source/WebInspectorUI/ChangeLog:9
> +	   Create a KeyboardShorcut for each of the following:

Nit: KeyboardShortcut

> Source/WebInspectorUI/UserInterface/Views/CanvasOverviewContentView.js:271
> +WI.CanvasOverviewContentView._itemMargin = 10;

I wish there was a sane way to share --variables between CSS and JS.. :|


More information about the webkit-reviews mailing list