Bug 181769

Summary:

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

Product:

WebKit

Reporter:

Joseph Pecoraro <joepeck>

Component:

Web Inspector

Assignee:

Devin Rousso <hi>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

agomez, commit-queue, ews-watchlist, hi, inspector-bugzilla-changes, mattbaker, timothy, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

All

OS:

All

Bug Depends on:

Bug Blocks:

175485

Attachments:

Description	Flags
Patch	none
[Image] After Patch is applied	none
Patch	none
[Image] After Patch is applied	none
[Image] Markup2.svg	none
[Image] Markup icons - 2 up	none
[Image] Markup icons - 2 up (Markup3.svg)	none
[Icon] Markup3.svg	none
Patch	none
[Image] After Patch is applied	none
[Image] New canvas buttons - close up	none

Joseph Pecoraro

Reported 2018-01-17 16:12:49 PST

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.

Attachments
Patch (9.38 KB, patch) 2018-01-17 21:03 PST, Devin Rousso	no flags	Details Formatted Diff Diff
[Image] After Patch is applied (38.59 KB, image/png) 2018-01-17 21:05 PST, Devin Rousso	no flags	Details
Patch (5.66 KB, patch) 2018-02-19 18:10 PST, Devin Rousso	no flags	Details Formatted Diff Diff
[Image] After Patch is applied (36.98 KB, image/png) 2018-02-19 18:11 PST, Devin Rousso	no flags	Details
[Image] Markup2.svg (421 bytes, image/svg+xml) 2018-02-19 20:34 PST, Matt Baker	no flags	Details
[Image] Markup icons - 2 up (589.08 KB, image/png) 2018-02-19 20:47 PST, Matt Baker	no flags	Details
[Image] Markup icons - 2 up (Markup3.svg) (589.22 KB, image/png) 2018-02-20 13:12 PST, Matt Baker	no flags	Details
[Icon] Markup3.svg (542 bytes, image/svg+xml) 2018-02-20 13:13 PST, Matt Baker	no flags	Details
Patch (5.48 KB, patch) 2018-02-20 17:46 PST, Devin Rousso	no flags	Details Formatted Diff Diff
[Image] After Patch is applied (105.85 KB, image/png) 2018-02-20 17:46 PST, Devin Rousso	no flags	Details
[Image] New canvas buttons - close up (171.33 KB, image/png) 2018-02-26 15:25 PST, Matt Baker	no flags	Details
Show Obsolete (5) View All Add attachment proposed patch, testcase, etc.

Devin Rousso

Comment 1 2018-01-17 21:03:26 PST

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

Comment 2 2018-01-17 21:05:00 PST

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

Comment 3 2018-01-18 11:47:28 PST

(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

Comment 4 2018-02-19 18:10:57 PST

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

Comment 5 2018-02-19 18:11:09 PST

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

Matt Baker

Comment 6 2018-02-19 20:31:27 PST

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

Comment 7 2018-02-19 20:34:23 PST

Created attachment 334232 [details] [Image] Markup2.svg Here's a polished </> icon. I used the Inspector's markup font as a guide.

Matt Baker

Comment 8 2018-02-19 20:47:05 PST

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

Comment 9 2018-02-20 09:29:17 PST

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

Comment 10 2018-02-20 12:33:51 PST

(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

Comment 11 2018-02-20 13:12:51 PST

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

Comment 12 2018-02-20 13:13:45 PST

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

Matt Baker

Comment 13 2018-02-20 13:14:49 PST

(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

Comment 14 2018-02-20 17:46:36 PST

Created attachment 334325 [details] Patch

Devin Rousso

Comment 15 2018-02-20 17:46:53 PST

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

Timothy Hatcher

Comment 16 2018-02-20 17:48:37 PST

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

Comment 17 2018-02-26 15:25:04 PST

(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

Comment 18 2018-02-26 15:25:32 PST

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

Timothy Hatcher

Comment 19 2018-02-26 15:26:12 PST

(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 20 2018-02-26 15:27:32 PST

Comment on attachment 334325 [details] Patch r=me

WebKit Commit Bot

Comment 21 2018-02-26 15:52:14 PST

Comment on attachment 334325 [details] Patch Clearing flags on attachment: 334325 Committed r229044: <https://trac.webkit.org/changeset/229044>

WebKit Commit Bot

Comment 22 2018-02-26 15:52:16 PST

All reviewed patches have been landed. Closing bug.

Radar WebKit Bug Importer

Comment 23 2018-02-26 15:53:20 PST

<rdar://problem/37920747>

Note You need to log in before you can comment on or make changes to this bug.