Web Inspector: DOM Node should have context menu to scroll it into view on the inspected page

• Component: Web Inspector
• Status: RESOLVED FIXED

[Joseph Pecoraro]

* SUMMARY
DOM Node should have context menu to scroll it into view on the inspected page

* NOTES
- Chrome and Firefox both have this.

[Joseph Pecoraro]

Created attachment 258780 [details]
[PATCH] Proposed Fix

[Devin Rousso]

Comment on attachment 258780 [details]
[PATCH] Proposed Fix

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

> Source/WebInspectorUI/UserInterface/Views/DOMTreeElement.js:1377
> +        var node = this.representedObject;

NIT: Aren't we using let now?

By the way, this doesn't apply to TOT.

[Timothy Hatcher]

Comment on attachment 258780 [details]
[PATCH] Proposed Fix

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

> Source/WebInspectorUI/UserInterface/Views/DOMTreeElement.js:672
> +            contextMenu.appendItem(WebInspector.UIString("Scroll Into View"), this._scrollIntoView.bind(this));

Should we only show this context menu item in the console and object trees? Showing it in the Elements tab seems silly.

[Nikita Vasilyev]

(In reply to comment #3)
> Comment on attachment 258780 [details]
> [PATCH] Proposed Fix
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=258780&action=review
> 
> > Source/WebInspectorUI/UserInterface/Views/DOMTreeElement.js:672
> > +            contextMenu.appendItem(WebInspector.UIString("Scroll Into View"), this._scrollIntoView.bind(this));
> 
> Should we only show this context menu item in the console and object trees?
> Showing it in the Elements tab seems silly.

I don't think it's silly. When I inspect CodeMirror I see lots of similar looking nodes — it's not always clear if the selected element is invisible or just out of the screen.

[Timothy Hatcher]

Oh, it scrolls the page not the DOM tree. Not silly at all.

[Timothy Hatcher]

Comment on attachment 258780 [details]
[PATCH] Proposed Fix

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

> Source/WebInspectorUI/UserInterface/Views/DOMTreeElement.js:670
> +        var node = this.representedObject;

Let here too.

[Joseph Pecoraro]

(In reply to comment #2)
> Comment on attachment 258780 [details]
> [PATCH] Proposed Fix
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=258780&action=review
> 
> > Source/WebInspectorUI/UserInterface/Views/DOMTreeElement.js:1377
> > +        var node = this.representedObject;
> 
> NIT: Aren't we using let now?
> 
> By the way, this doesn't apply to TOT.

This doesn't apply to ToT because of localizedString changes and svn-apply doesn't deal with binary diffs well.

[Joseph Pecoraro]

http://trac.webkit.org/changeset/188343