1419086 - Render the ObjectInspector in the sidebar

Status: RESOLVED FIXED

(DevTools :: Console, enhancement, P2)

Description

[Nicolas Chevobbe [:nchevobbe]]

We should render a ObjectInspector component corresponding to the clicked object in the sidebar

Comment 1

[Mike Park [:mparkms]]

Attachment: Bug 1419086 - Render the ObjectInspector in the console sidebar.

Review commit: https://reviewboard.mozilla.org/r/207628/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/207628/

Comment 2

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8936906 [details]
Bug 1419086 - Render the ObjectInspector in the console sidebar.

https://reviewboard.mozilla.org/r/207628/#review213512

This looks good, thanks Mike !
Seeing the way we insantiate the ObjectInspector, I fear we missed something either in the sidebar, or in GripmessageBody when we need to add other props.
Do you think we could have a getObjectInspector(grip, serviceContainer, override) function ?
Overrides would let us specify different things in GripMessageBody and Sidebar (like autoExpand for example).
This would then be easier to use the new functionalities of ObjectInspector

::: devtools/client/webconsole/new-console-output/components/SideBar.js:50
(Diff revision 1)
> +      ? (object) => serviceContainer.highlightDomElement(object)
> +      : null;

this should be indented

::: devtools/client/webconsole/new-console-output/components/SideBar.js:54
(Diff revision 1)
> +      ? (object, e) => {
> +        // Stop the event propagation so we don't trigger ObjectInspector expand/collapse.
> +        e.stopPropagation();
> +        serviceContainer.openNodeInInspector(object);
> +      }
> +      : null;

this should be indented

Comment 3

[Mike Park [:mparkms]]

Comment on attachment 8936906 [details]
Bug 1419086 - Render the ObjectInspector in the console sidebar.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/207628/diff/1-2/

Comment 4

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8936906 [details]
Bug 1419086 - Render the ObjectInspector in the console sidebar.

https://reviewboard.mozilla.org/r/207628/#review213956

::: devtools/client/webconsole/new-console-output/components/SideBar.js:43
(Diff revisions 1 - 2)
>        sidebarVisible,
>        grip,
>        serviceContainer,
>      } = this.props;
>  
> -    let onDOMNodeMouseOver;
> +    let override = {

i think i can inline this object directly in the getObjectInspector call

::: devtools/client/webconsole/new-console-output/components/GripMessageBody.js:65
(Diff revision 2)
>    let styleObject;
>    if (userProvidedStyle && userProvidedStyle !== "") {
>      styleObject = cleanupStyle(userProvidedStyle, serviceContainer.createElement);
>    }
>  
> -  let onDOMNodeMouseOver;
> +  let override = {

we could name it objectInspectorProps I think, so it is more obious what it is.

::: devtools/client/webconsole/new-console-output/utils/object-inspector.js:1
(Diff revision 2)
> +/* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */
> +/* vim: set ft=javascript ts=2 et sw=2 tw=80: */

let's not add those

::: devtools/client/webconsole/new-console-output/utils/object-inspector.js:81
(Diff revision 2)
> +  Object.assign(objectInspectorProps, override);
> +
> +  return ObjectInspector(objectInspectorProps);

I think we can do 
return ObjectInspector({...objectInspectorProps, override});

Comment 5

[Mike Park [:mparkms]]

Comment on attachment 8936906 [details]
Bug 1419086 - Render the ObjectInspector in the console sidebar.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/207628/diff/2-3/

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c82766bc8524
Render the ObjectInspector in the console sidebar. r=nchevobbe

Comment 7

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/c82766bc8524