Open
Bug 1590392
Opened 5 years ago
Updated 2 years ago
Cleanup css-logic modules found in devtools/shared and devtools/server/actors
Categories
(DevTools :: Inspector, task, P3)
DevTools
Inspector
Tracking
(Not tracked)
NEW
People
(Reporter: jdescottes, Unassigned)
References
Details
(See the comment at https://phabricator.services.mozilla.com/D49330?id=179492#inline-303983 for more context)
Today we have two modules called css-logic:
- one in devtools/shared: https://searchfox.org/mozilla-central/rev/55aa17110091deef24b913d033ccaf58f9c6d337/devtools/shared/inspector/css-logic.js
- one in devtools/server/actors: https://searchfox.org/mozilla-central/rev/55aa17110091deef24b913d033ccaf58f9c6d337/devtools/server/actors/inspector/css-logic.js
The devtools/shared/inspector css-logic module is mostly made of static CSS-related helpers. Those helpers are mainly used in the server (exception in text-property-editor.js). This module also exposes the styleeditor localization helper as l10n
which a few modules use as a shortcut.
The tests for this module are found at devtools/shared/tests/mochitest and are chrome mochitests. https://searchfox.org/mozilla-central/source/devtools/shared/tests/mochitest
The devtools/server/actors css-logic module exposes mostly 2 things:
- a CssLogic class that has to be instantiated, and that keeps a sort of state depending on which "element" it currently highlights. As the JsDoc mentions, part of the class depends on the currently highlighted element, part doesn't...
- some static helpers exposed on CssLogic's constructor (eg getBindingElementAndPseudo). They are typically imported from devtools/shared/inspector/css-logic.js and just exposed via an alias here
The goals of this bug are:
- move all the static helpers to a consistent location and stop aliasing them on the actor's file
- potentially split the helpers in several modules if it makes sense. It seems like we could have one module about selectors & paths, one module about CSS prettifying...
- make sure the tests are right next to the implementation
- find a more suitable name for the CssLogic class. It really isn't clear what this class is supposed to do. If some methods in the class are actually not relying on the state of the instance and can be moved to static helpers, move them out.
- potentially split out the subclasses defined in actors' css-logic.js in dedicated modules
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•