Bug 178800

<rdar://problem/35175856>

Created attachment 324974 [details] Patch

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.. :|

(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 :)

(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

Created attachment 325066 [details] Patch

Comment on attachment 325066 [details] Patch r=me, thanks Devin!

Comment on attachment 325066 [details] Patch Clearing flags on attachment: 325066 Committed r224067: <https://trac.webkit.org/changeset/224067>

All reviewed patches have been landed. Closing bug.