Web Inspector: Command-Enter in Debugger should show a popover with evaluation result

• Component: Web Inspector
• Status: RESOLVED FIXED

[Nikita Vasilyev]

I've been using Command-Enter in Debugger (https://bugs.webkit.org/show_bug.cgi?id=148368) for a few days and I'm not very satisfied.

I think Command-Enter should show the exact same popover as on hover. The current behavior, evaluating in the console, is too disconnected 
from the source code and requires an extra mental effort to keep the context.

[Radar WebKit Bug Importer]

<rdar://problem/22710660>

[Nikita Vasilyev]

Created attachment 261257 [details]
[Animated GIF] With the patch applied

Command-Enter shows the popover.

[Nikita Vasilyev]

Created attachment 261258 [details]
[Animated GIF] Cmd-Enter shows type info popovers

I also accidentally improved keyboard navigation for type profiler. Command-Enter shows popovers with type information too.

[Nikita Vasilyev]

Created attachment 261259 [details]
WIP

[Nikita Vasilyev]

Created attachment 261305 [details]
Patch

Currenty, the only way to display the popover is to hover over the text with a mouse cursor. Provide a way to display it via a keyboard shortcut, Command-Enter.

[Nikita Vasilyev]

Created attachment 261306 [details]
Patch

[BJ Burg]

Comment on attachment 261306 [details]
Patch

Oops, this patch has been gathering dust far too long, sorry about that. I don't know the CodeMirror integrations well enough to review this. Joe or Tim (or Devin), can you take a look?

[Joseph Pecoraro]

Comment on attachment 261306 [details]
Patch

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

> Source/WebInspectorUI/UserInterface/Controllers/CodeMirrorTokenTrackingController.js:593
> +WebInspector.CodeMirrorTokenTrackingController.TriggeredBy = {
> +    Keyboard: "keyboard",
> +    Hover: "hover"
> +};

Seems this would add complexity for a very hard to discover feature. Is there some way we can make this more discoverable, otherwise I fear it wouldn't be used much?

[Nikita Vasilyev]

(In reply to comment #8)
> Comment on attachment 261306 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=261306&action=review
> 
> > Source/WebInspectorUI/UserInterface/Controllers/CodeMirrorTokenTrackingController.js:593
> > +WebInspector.CodeMirrorTokenTrackingController.TriggeredBy = {
> > +    Keyboard: "keyboard",
> > +    Hover: "hover"
> > +};
> 
> Seems this would add complexity for a very hard to discover feature. Is
> there some way we can make this more discoverable, otherwise I fear it
> wouldn't be used much?

I don't think it's that different from the rest of keyboard shortcuts. Also, Command-Enter already evaluates selected console message: https://twitter.com/ELV1S/status/636431257379864576.

Eventually, we will have a page with all shortcuts. I'll also write a blog post, too.

I think this shortcut isn't that hard to remember. Certainly easier than Chrome's Control-Shift-E, which evaluates selection in the console.

I don't think we should give up on keyboard accessability only because it adds complexity. This patch levereges existing mouse over handling.

I had to add WebInspector.CodeMirrorTokenTrackingController.TriggeredBy.Keyboard to avoid hiding the popover on mouse move. When we show the popover on mouse hover, we hide it when there is a mouse movement outside of evaluated token. It didn't work well with Command-Enter and I had to add WebInspector.CodeMirrorTokenTrackingController.TriggeredBy.Keyboard.

[Nikita Vasilyev]

Created attachment 263793 [details]
Patch

Rebaselined.

[WebKit Commit Bot]

Comment on attachment 263793 [details]
Patch

Clearing flags on attachment: 263793

Committed r191496: <http://trac.webkit.org/changeset/191496>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.