Bug 181769 – Web Inspector: Canvas Tab: Scroll into view / Inspect element if Canvas has DOM node

• Component: Web Inspector
• Status: RESOLVED FIXED

[Joseph Pecoraro]

ER: Canvas Tab: Scroll into view / Inspect element if Canvas has DOM node

There are already a bunch of quick actions in the Overview to record / refresh. I'd like to see an action to try to scroll the canvas into view on the real page / highlight the element on the page if possible. These kinds of things.

[Devin Rousso]

Created attachment 331587 [details]
Patch

I'm gonna put it out there and say that I don't like the icons I used for Log and the Canvas Element menu, but I'm not sure what else to use.  This is more of a proof of concept.  Feedback would be greatly appreciated.

[Devin Rousso]

Created attachment 331588 [details]
[Image] After Patch is applied

The image for "Log Canvas Context" is the same icon we use for `console.log()` results.
The image for "Canvas Element" is the icon we use for elements in the DOM tree, and the context menu that appears when you click on it is the same as what you would get in the DOM tree.

[Matt Baker]

(In reply to Devin Rousso from comment #1)
> Created attachment 331587 [details]
> Patch
> 
> I'm gonna put it out there and say that I don't like the icons I used for
> Log and the Canvas Element menu, but I'm not sure what else to use.  This is
> more of a proof of concept.  Feedback would be greatly appreciated.

What about a simple monochrome "</>" icon for the element menu? I think that would look better alongside the other two buttons. Also, let's drop the Log button since Log Element can be accessed from the context menu.

[Devin Rousso]

Created attachment 334219 [details]
Patch

The Markup.svg icon is what I was able to come up with in Sketch.  Definitely could use a better icon -.-

[Devin Rousso]

Created attachment 334220 [details]
[Image] After Patch is applied

[Matt Baker]

Comment on attachment 334219 [details]
Patch

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

> Source/WebInspectorUI/UserInterface/Views/CanvasContentView.js:339
> +                const text = WI.UIString("Selected Canvas Context");

Nit: I don't find this const particularly helpful, but I'll leave it up to you.

[Matt Baker]

Created attachment 334232 [details]
[Image] Markup2.svg

Here's a polished </> icon. I used the Inspector's markup font as a guide.

[Matt Baker]

Created attachment 334234 [details]
[Image] Markup icons - 2 up

I'm going to tweak the new icon a bit. I think it looks too compact.

[Timothy Hatcher]

Matt, I do like the second icon better, but it seems a little too bold. (Maybe it is the screenshot not being viewed as HiDPI.) Compared to the reload icon weight, it is just a hair more it seems.

[Matt Baker]

(In reply to Devin Rousso from comment #5)
> Created attachment 334220 [details]
> [Image] After Patch is applied

My only other comment is that I think the '</>' button should be last. Having to maneuver the cursor between two buttons to hit Record seems bad.

[Matt Baker]

Created attachment 334298 [details]
[Image] Markup icons - 2 up (Markup3.svg)

- Fixed SVG path color
- Increased height of brackets and slash
- Increased angle of brackets

[Matt Baker]

Created attachment 334299 [details]
[Icon] Markup3.svg

[Matt Baker]

(In reply to Timothy Hatcher from comment #9)
> Matt, I do like the second icon better, but it seems a little too bold.
> (Maybe it is the screenshot not being viewed as HiDPI.) Compared to the
> reload icon weight, it is just a hair more it seems.

I had forgotten to set paths to use fill="currentColor".

[Devin Rousso]

Created attachment 334325 [details]
Patch

[Devin Rousso]

Created attachment 334326 [details]
[Image] After Patch is applied

[Timothy Hatcher]

Comment on attachment 334325 [details]
Patch

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

> Source/WebInspectorUI/UserInterface/Images/Markup.svg:6
> +    <path fill="currentColor" d="M 15.5 8 a 0.5 0.5 0 0 1 -0.24 0.43 L 11 10.99 V 9.82 L 14.03 8 11 6.18 V 5.01 l 4.26 2.56 A 0.5 0.5 0 0 1 15.5 8 Z"/>
> +    <polygon fill="currentColor" points="10.77 3 6.32 13 5.23 13 9.68 3 10.77 3"/>
> +    <path fill="currentColor" d="M 5 6.18 1.97 8 5 9.82 v 1.17 L 0.74 8.43 a 0.505 0.505 0 0 1 0 -0.86 L 5 5.01 Z"/>

Is it me, or is the color not matching the reload icon? The reload icon looks like a grey, where the arrows look black.

[Matt Baker]

(In reply to Timothy Hatcher from comment #16)
> Comment on attachment 334325 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=334325&action=review
> 
> > Source/WebInspectorUI/UserInterface/Images/Markup.svg:6
> > +    <path fill="currentColor" d="M 15.5 8 a 0.5 0.5 0 0 1 -0.24 0.43 L 11 10.99 V 9.82 L 14.03 8 11 6.18 V 5.01 l 4.26 2.56 A 0.5 0.5 0 0 1 15.5 8 Z"/>
> > +    <polygon fill="currentColor" points="10.77 3 6.32 13 5.23 13 9.68 3 10.77 3"/>
> > +    <path fill="currentColor" d="M 5 6.18 1.97 8 5 9.82 v 1.17 L 0.74 8.43 a 0.505 0.505 0 0 1 0 -0.86 L 5 5.01 Z"/>
> 
> Is it me, or is the color not matching the reload icon? The reload icon
> looks like a grey, where the arrows look black.

Looks okay to me.

[Matt Baker]

Created attachment 334653 [details]
[Image] New canvas buttons - close up

[Timothy Hatcher]

(In reply to Matt Baker from comment #18)
> Created attachment 334653 [details]
> [Image] New canvas buttons - close up

That looks right. Thanks!

[Matt Baker]

Comment on attachment 334325 [details]
Patch

r=me

[WebKit Commit Bot]

Comment on attachment 334325 [details]
Patch

Clearing flags on attachment: 334325

Committed r229044: <https://trac.webkit.org/changeset/229044>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/37920747>