[Webkit-unassigned] [Bug 177690] Web Inspector: Layers tab sidebar's DOM highlight should be by row hover, not row selection

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=177690

Devin Rousso <webkit at devinrousso.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #322263|review?                     |review+
              Flags|                            |

--- Comment #6 from Devin Rousso <webkit at devinrousso.com> ---
Comment on attachment 322263
  --> https://bugs.webkit.org/attachment.cgi?id=322263
Patch

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

r=me.  Looks good, with a few minor comments.

> Source/WebInspectorUI/UserInterface/Views/LayerDetailsSidebarPanel.js:96
> +        this._dataGrid.element.addEventListener("mouseout", this._dataGridMouseOut.bind(this), false);

I think you want to use "mouseleave" instead.  "mouseout" will fire every time the mouse leaves any child element, of which there are a lot in DataGrid.  This case is already covered by your "mousemove" implementation, so you only need to add a listener for when the mouse leaves the DataGrid itself, not any of its children.

<https://developer.mozilla.org/en-US/docs/Web/Events/mouseleave>

> Source/WebInspectorUI/UserInterface/Views/LayerDetailsSidebarPanel.js:139
> +        if (node === this._hoveredDataGridNode)

This check should probably happen before the if on (134), as otherwise we are firing unnecessary `WI.domTreeManager.hideDOMNodeHighlight` calls.

    let node = this._dataGrid.dataGridNodeFromNode(event.target);
    if (node === this._hoveredDataGridNode)
        return;

    if (!node) {
        this._hideDOMNodeHighlight();
        return;
    }

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20171002/fde69f29/attachment.html>