On hover variations between classes and functions in the Outline panel

[DPX-designer]

On hover, the functions and classes present in the Outline panel receive a background color change, we also see that these are visualised differently:

Hover over a function:

Hover over a class:

The margin applied to the class name causes a blank space to appear left of the class keyword, variations in padding also reduces the breathing room the class labels have.

The indent of the background hover is odd as the methods belonging to that class have a full width hover despite them being children of that class, so if anything being the other way around would make more sense. Having them all with a full width hover background seems consistent with other panels though and would probably look more "correct".

[fvsch]

We can probably bring the class name and its methods a bit closer, too. So that the content reads as:

class Something
  λ someMethod
  λ otherMethod

class AnotherThing
  λ constructor(...)
  λ doSomething

[yogmel]

Hi all,
Can I work on this? Both class and function hover should look consistent, right?

Thanks!

[fvsch]

Can I work on this?

That would be great! I think you can make a pull request on the https://github.com/firefox-devtools/debugger repository directly, without opening an issue there. Your commit message and/or PR title could reference this issue directly.

Both class and function hover should look consistent, right?

Yep. One thing to look out for perhaps: in some examples I've seen class Foo headers which had the hover style, but clicking them did nothing. If there are reasons why they don't work in some instances, it would be best to not have the hover style in those instances.
@darkwing Is this something you've seen before?

[yogmel]

Thanks @fvsch! I'll have a look today and will get back here with updates very soon :)

[yogmel]

Hi all,
I have tested a little bit and came up with this:

Let me know what do you think!

[fvsch]

That looks pretty good.

I would try to improve a few things in addition to that:

1. Can we have the top margin before class Foo only between two blocks? When functions are declared at the root level (not in a class), they appear something like 4px below the search filter. It would be nice to keep the same spacing if the first element in the list is a class Something declaration. This can probably be done by only adding a margin-top to .outline-list__class:not(:first-child).

2. It would be great to change .outline-list__class from a <div> to a <li> element. Right now we're outputting this kind of DOM, which is not valid:

<ul class="outline-list devtools-monospace">
  <li class="outline-list__element">...</li>
  <div class="outline-list__class">...</div>
</ul>

3. Could we make it so that functions declared at the top level (rather than as class methods) are not indented? It would look like this:

Sorry if I'm adding to your plate. We can address those separately if you prefer, and just focus on the hover background for now.

[yogmel]

Hi @fvsch ,
Thanks for your feedback!

It's fine! I have worked on it. Let me know what you think:

In the gif above, the first class bbb has a margin-top, because it's not the first element to appear (it's eee()).

When the class is the first element, it doesn't have any spacing on top:

Also, I believe part of your message got erased. But maybe I understood what you said. It's about the list items? I have changed the div to be a list item, so the structure is:

<ul class="outline-list devtools-monospace">
    <li class="outline-list__element"></li>
    <li class="outline-list__class">
      <h2></h2>
      <ul class="outline-list__class-list">
        <li class="outline-list__element"></li>
      </ul>
    </li>
  </ul>

I've changed on this file.

[fvsch]

That looks perfect. Are you ready to create a pull request on the debugger repo? We can continue the review there.

[yogmel]

Hi @fvsch , I've just created a PR, here:
firefox-devtools/debugger#8126
Thanks!