1381819 - Polish console.group/ObjectInspector styling to be more consistent

Status: RESOLVED FIXED

(DevTools :: Console, enhancement, P1)

Description

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Screen Shot 2017-07-18 at 14.30.18.png

Currently, the styles of different expandable object in the console (mainly console.group and objectInspector) are not really consistent.
We should make sure we get alignment and colors right before rolling out the new console in all channel.

Comment 1

[Nicolas Chevobbe [:nchevobbe]]

Also, we should make the whole group message clickable to be consistent with what we have in the object inspector. Currently, you need to click on the arrow, which is a small target.

Comment 2

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1381819 - Align exception/xhr/group arrows with ones in ObjectInspector; .

We now use the same image for the arrow, and the console component which render
this arrow was changed for consistency with the objectInspector naming.
Some alignment properties needed to be changed so everything lines up perfectly.
Tests were updated to match the new arrow classnames.

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

Comment 3

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Screenshot_patch_applied

Here's how it looks like with the patch

Comment 4

[Brian Grinstead [:bgrins]]

Comment on attachment 8962409 [details]
Bug 1381819 - Align exception/xhr/group arrows with ones in ObjectInspector; .

https://reviewboard.mozilla.org/r/231262/#review236668

::: devtools/client/themes/webconsole.css:1216
(Diff revision 1)
>  .webconsole-output-wrapper .object-inspector.tree .tree-node:hover:not(.focused) {
>    background-color: var(--object-inspector-hover-background);
>  }
>  
>  /*
>   * Make the arrow the same color and approximately the same size of the twisty icon.

As discussed, can you close this bug and remove the comment? We also mentioned adding a comment that these styles are adapted from reps.css.

::: devtools/client/themes/webconsole.css:1219
(Diff revision 1)
>  
>  /*
>   * Make the arrow the same color and approximately the same size of the twisty icon.
>   * Should be properly fixed in https://bugzilla.mozilla.org/show_bug.cgi?id=1307937.
>   */
> +.webconsole-output-wrapper .message > img.arrow {

We discussed adding a second class onto the CollapseButton arrow so we could change selectors like:

`.webconsole-output-wrapper .message > img.arrow`

to something like:

`.webconsole-output-wrapper > img.arrow.collapse-button`

To make it more clear that this isn't intended to hit OI arrows

::: devtools/client/themes/webconsole.css:1231
(Diff revision 1)
> +  margin-inline-end: 1px;
> +  transform: rotate(-90deg);
> +  transition: transform 0.125s ease;
> +}
> +
> +html[dir="rtl"] .webconsole-output-wrapper img.arrow {

This should target only CollapseButton arrows

::: devtools/client/themes/webconsole.css:1235
(Diff revision 1)
> +
> +html[dir="rtl"] .webconsole-output-wrapper img.arrow {
> +  transform: rotate(90deg);
> +}
> +
> +.webconsole-output-wrapper img.arrow.expanded.expanded {

Ditto. I'm also not a huge fan of .expanded.expanded here and would prefer the previous selector was something like `html[dir="rtl"] .webconsole-output-wrapper img.arrow:not(.expanded)`, but I understand this is copied in from reps so we could handle refactoring that separately.

::: devtools/client/themes/webconsole.css:1239
(Diff revision 1)
> +
> +.webconsole-output-wrapper img.arrow.expanded.expanded {
> +  transform: rotate(0deg);
> +}
> +
> +.webconsole-output-wrapper .arrow,

As discussed: double check that this covers both OI and CollapseButtons with `.webconsole-output-wrapper img.arrow` and remove the second selector here if so.

::: devtools/client/themes/webconsole.css:1244
(Diff revision 1)
> +.webconsole-output-wrapper .arrow,
>  .webconsole-output-wrapper .object-inspector.tree .tree-node .arrow {
>    background-color: #AFA8AB;
>  }
>  
> +.theme-dark .webconsole-output-wrapper .arrow,

Ditto

Comment 6

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8962409 [details]
Bug 1381819 - Align exception/xhr/group arrows with ones in ObjectInspector; .

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

Comment 7

[Brian Grinstead [:bgrins]]

Comment on attachment 8962409 [details]
Bug 1381819 - Align exception/xhr/group arrows with ones in ObjectInspector; .

https://reviewboard.mozilla.org/r/231262/#review237554

::: devtools/client/themes/webconsole.css:1217
(Diff revision 2)
>    background-color: var(--object-inspector-hover-background);
>  }
>  
>  /*
> - * Make the arrow the same color and approximately the same size of the twisty icon.
> - * Should be properly fixed in https://bugzilla.mozilla.org/show_bug.cgi?id=1307937.
> + * Make console.group, exception and XHR message's arrow look the same as the arrow
> + * used in the ObjectInspector (same background-image, width, transition).

Nit: include the fact that these are copied in from reps.css

Comment 8

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8962409 [details]
Bug 1381819 - Align exception/xhr/group arrows with ones in ObjectInspector; .

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

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d1b2a8d328b9
Align exception/xhr/group arrows with ones in ObjectInspector; r=bgrins.

Comment 10

[Raul Gurzau (:RaulG)]

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