1413678 - Auto-suggest classes while adding new classes to elements

Status: RESOLVED DUPLICATE of bug 1492797

(DevTools :: Inspector, enhancement, P3)

Description

[Stephanie Hobson [:shobson]]

In the inspector you can select the `.cls` button to add classes to elements. Neat! 

In Chrome it also auto-suggests classes as you type. That would be cool too.

Comment 1

[yohanmishkin]

I'd love to take a hack at this if there's likelihood a PR for this enhancement would get merged.

Comment 2

[Patrick Brosset <:pbro>]

Yes, we would gladly accept code for this feature. Thank you for your interest.
Let me try to give you some information to help you get started on this.

First off, our contributors documentation lives here: https://docs.firefox-dev.tools/
You'll need to go through it to know how to get the source code, build it, make changes, submit code, etc.

Next, let me give you some pointers into the code:
- The .cls button and its associated input field UI are mostly handled in this file: /devtools/client/inspector/rules/views/class-list-previewer.js (which you can see online on searchfox: https://searchfox.org/mozilla-central/source/devtools/client/inspector/rules/views/class-list-previewer.js)
- The ClassListPreviewer class in this file is responsible for the input element (this.addEl refers to it)
- Now, to actually display a suggestion list when typing, the easiest would be to reuse the same component we use elsewhere in devtools (e.g. to auto-complete function names in the console, or property names in the CSS Rules panel). This component is in https://searchfox.org/mozilla-central/source/devtools/client/shared/autocomplete-popup.js
You can see various usage examples for this component here: https://searchfox.org/mozilla-central/search?q=new+AutocompletePopup&path=devtools
- Finally, we need to get the data for listing the classes in the popup. I guess there are several ways to do this. Looking at how Chrome DevTools does it, they send a request to their backend, iterating over each and every stylesheet in the doc, and from there, getting each and every rule, extracting classes from their selectors. They limit this to the frame where the current node lives however (no need to get the classes from stylesheets that are loading in child iframes for instance).
I think this is the right behavior to have too, since that will give all possible classes you can use on an element, because they exist in a stylesheet you have access to.
This feature will need to be added to the devtools backend (aka the actor). In this case, I think the best fit is the StyleSheetActor which lives in /devtools/server/actors/stylesheets.js. We need to add a getAllClasses method to this class. Which the autocomplete will need to call.

There's still some details omitted here, but if people are interested in fixing this bug, this should provide enough entry points.
Note: this isn't a really easy first bug. It will be a bit complex and involve multiple of the layers of the DevTools architecture. But it's all JavaScript only. And a really good opportunity to understand more about how things work.
I'll set myself as a mentor on the bug too, which basically mean: don't worry if you don't know the answers or how to proceed, feel free to ask any questions you want here in the bug.

Comment 3

[yohanmishkin]

Awesome! Thanks so much for the detailed response, Patrick! Hoping to dig in later this week!

Comment 4

[Vincent]

Hi,
I would like to work on this if possible.
[e-mail], are you still actively working on this? Thanks!

Comment 5

[Jefry Lagrange]

Nobody is working on this currently?

Comment 6

[Vincent]

I was planning to work on it (this week/weekend).
[e-mail] did not answer so I guess it's ok to take it over.

Comment 7

[Patrick Brosset <:pbro>]

Assigning the bug to Vincent now.

Comment 8

[yohanmishkin]

Hey Vincent! Thanks for picking this up! And sorry for not responding. Let me know if there's anything I can do to help or if you'd like to pair on any of it. 

Thanks again!

Comment 9

[Patrick Brosset <:pbro>]

Hi Vincent,
Are you still interested in working on this bug?
It is assigned to you now but hasn't been updated for 2 months.
If you need more time, or help, no worries, let's keep it assigned and feel free to reach out to me for help.
If, however, you don't think you'll be working on this bug, please let me know so I can unassign you from it and so it becomes available to others.
Thanks!

Comment 10

[Patrick Brosset <:pbro>]

Unassigning now.

Comment 11

[Thomas]

Hi,
I would like to work on that bug!

Comment 12

[Patrick Brosset <:pbro>]

Thanks Thomas. Assigning the bug to you now. Let me know if you have any questions on top of what I explained in comment 2 (which was last year, so might have become a tad outdated).

Comment 13

[Thomas]

Attachment: Bug 1413678 - start auto suggestion in class list previewer. r=pbro

Auto suggest class names in class list previewer based on loaded stylesheets

Comment 14

[Patrick Brosset <:pbro>]

(In reply to Patrick Brosset <:pbro> from comment #2)

• Finally, we need to get the data for listing the classes in the popup. I
  guess there are several ways to do this. Looking at how Chrome DevTools does
  it, they send a request to their backend, iterating over each and every
  stylesheet in the doc, and from there, getting each and every rule,
  extracting classes from their selectors. They limit this to the frame where
  the current node lives however (no need to get the classes from stylesheets
  that are loading in child iframes for instance).
  I think this is the right behavior to have too, since that will give all
  possible classes you can use on an element, because they exist in a
  stylesheet you have access to.

I looked again and they also list the classes that exist in the DOM.
Try opening this page in Chrome:
data:text/html,<style>.ss-class-1{color:red}.ss-class-2{color:blue}</style><div class="dom-class-1"></div><div class="dom-class-2"></div>
And try to add new classes to an element using the .cls UI, you'll see all classes are available in the autocomplete, even those found in the DOM and not in a stylesheet.

However if we want to take this in steps, listing the classes found in stylesheets is enough as a first step. This is probably the most useful thing to do anyway.

Thinking of implementation details, relying on StylesheetActors is probably not the right thing to do because those only get instantiated on demand, when needed. So the style-editor does this, but the feature discussed here should work even if you don't open the style-editor first. The inspector also instantiates these actors, but only when needed. However here we need something that returns all classes from all stylesheets loaded in a given document even if they haven't been seen by the inspector or style-editor.

So we do need a global method that returns this data. I think the approach in D34762 to add this to the StylesheetsActor instead was the right one.
However we need to restrict this to the document where the current element exists. Currently we rely on getStyleSheets in the patch but this goes into all windows. Instead we should probably use _addStyleSheets.
In fact, maybe we instead need to add a new method to the NodeActor class instead, so it's more intuitive that it works in the context of a document only. So from the front-end we would call selection.nodeFront.getAllClasses and the getAllClasses method would be added to the class in this file instead.

This method would do something like this:

const styleSheets = InspectorUtils.getAllStyleSheets(this.rawNode.ownerDocument);
for (let i = 0; i < styleSheets.length; i++) {
  const sheet = styleSheets[i];
  const actor = targetActor.createStyleSheetActor(sheet);
  actor.getAllClasses();
}

This means we would implement the listing of all selectors and parsing of classnames in StylesheetActor.

I think of 2 use cases that we might want to worry about now rather than later:

• we also want autocomplete when adding a new rule, so limiting this to getAllClasses will become a problem in the future. It might be good to make the method name more generic so we can extend it in the future without breaking backward compatibility.
• there is no need to return all classes if the user has already started to type abc in the input field. We could do the filtering on the actor side, instead of the front-end.

Based on these 2 things, I think we should call the new method suggestSelectorPart(query, type). Where query would be an optional string to filter the classes or ids. And type would be class or id (or something else in the future).

If we start by adding this method to StylesheetActor as a first step, that would be a great first step. Then we can think of what's next and how it all fits into the feature.

Comment 15

[Simon Walker]

Hi all, how is progress going on this bug? This is possibly a duplicate of one I've just volunteered to help with #1492797, and I'd like to join forces.

Comment 16

[Patrick Brosset <:pbro>]

Unassigning as Thomas said he didn’t have time to work on it just now. Simon, are you still interested?

Comment 17

[Simon Walker]

I am still interested. I was looking along these lines for my bug so merging the two may make sense.

Comment 18

[aarushivij]

Hey can I work on this bug?
Aarushi