Bug 178800 – Web Inspector: Canvas Tab: canvases overview should support navigation via keyboard

• Component: Web Inspector
• Status: RESOLVED FIXED

[Blaze Burg]

Expected:

- left/up moves to previous canvas
- right/down moves to next canvas
- space bar starts/stops a recording

Actual:

Nothing works

[Radar WebKit Bug Importer]

<rdar://problem/35175856>

[Devin Rousso]

Created attachment 324974 [details]
Patch

[Blaze Burg]

Comment on attachment 324974 [details]
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.. :|

[Devin Rousso]

(In reply to Brian Burg from comment #3)
> Comment on attachment 324974 [details]
> 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.)
I think it might be useful, but at the same time there is no real reason to support selection for a general CollectionContentView unless you can do something with that collection.  As an example, selecting an item for the Images folder in Resources doesn't really do anything since there is no recording (or anything really) feature there.

> - RTL would change visual order but not the logical order. Can you make sure
> that these shortcuts work as intended?
I'll have to check on that and make sure it works.

> > 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.. :|
I think I have a way to do this :)

[Nikita Vasilyev]

(In reply to Devin Rousso from comment #4)
> > > 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.. :|
> I think I have a way to do this :)

This is how it's usually done:
https://developer.mozilla.org/en-US/docs/Web/CSS/Using_CSS_variables#Values_in_Javascript

[Devin Rousso]

Created attachment 325066 [details]
Patch

[Blaze Burg]

Comment on attachment 325066 [details]
Patch

r=me, thanks Devin!

[WebKit Commit Bot]

Comment on attachment 325066 [details]
Patch

Clearing flags on attachment: 325066

Committed r224067: <https://trac.webkit.org/changeset/224067>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.