Skip to content

Instantly share code, notes, and snippets.

@captainbrosset
Last active July 11, 2016 15:41
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save captainbrosset/937fb40a95b59cfe5e8496b16bccde86 to your computer and use it in GitHub Desktop.
Save captainbrosset/937fb40a95b59cfe5e8496b16bccde86 to your computer and use it in GitHub Desktop.
Refactoring the way CSS declarations are displayed in the rule-view to better support visual edition tools

Refactoring the way CSS declarations are displayed in the rule-view to better support visual edition tools

Goal

As part of the effort to create more visual edition tools in the rule-view (see bug 1258390), we need to be able to easily add new functionality to how CSS declarations are displayed and interacted with in the css rule-view.

Not only do we want it to be easy for us to add new types of editors (for gradients, borders, shadows, grids etc.), but we also want addon authors to be able to extend the rule-view too.

CSS declarations (name:value; pairs) are the most important part of the rule-view and represent what people interact with the most. This is where we currently show things like color swatches, linkify URLs to background images, offer ways to edit animation timing-functions, and more.

Right now, generating the DOM for a CSS declaration is handled by the OutputParser, which contains very specific code that detects colors and timing-functions and filters and such.

Adding more to the OutputParser is not trivial, and there is no API in place to easily extend the current functionality. Plus, changing the OutputParser isn't nearly enough, indeed color picker tooltips, for example, are handled by a mix of 2 other things: the TooltipsOverlay and the TextPropertyEditor.

So the current situation is complex and not easily extensible.

High level architecture proposal

Below is a proposal to make the situation better. It will be implemented in bug 1275029

Integration in the rule-view

CSS declarations are handled by the TextPropertyEditor today. We don't intend to change this, the main structure of the rule-view remains unchanged.

The proposal only aims at getting rid of the code that generates the DOM in the TextPropertyEditor and replacing the OutputParser with a new system.

Parsing declarations

CSS declarations are parsed by using the CSS lexer to produce a series of tokens and then looping through them to return a CSS AST.

The goal here isn't to produce a standard AST, but rather one that contains data that makes sense for the rule-view. Each node in the tree would be something potentially useful for the rule-view, like a url, a color, a function, etc.

As an example, for filter: hue-rotate(180deg) blur(2px), the high-level AST structure will be:

declaration
  + name (filter)
  + values
    + value
      + function (hue-rotate)
        + argument
          + dimension (number: 180, unit: deg)
      + function (blur)
        + argument
          + dimension (number: 2, unit: px)

Or for background: url(1.png), linear-gradient(12deg, blue, red), #f06; the AST will look something like:

declaration
  + name (background)
  + values
    + value
      + function (url)
        + argument
          + url (1.png)
    + value
      + function (linear-gradient)
        + argument
          + dimension (number: 12, unit: deg)
        + argument
          + color (blue)
        + argument
          + color (red)
    + value
      + color (#f06)

Not only are there leaf nodes useful for handling various CSS types like colors or dimensions for examples, but there are also value, function or argument nodes, which are useful to do things like handling edition with the InplaceEditor, or formatting.

Handling tree nodes

Once the AST for a declaration is ready, it is being visited, and each node is then passed to handlers. Handlers are the actual code that generate the DOM output for a declaration.

It is be possible to register new handlers so that we can easily extend the functionaility later, and allow addons to add handlers too.

A handler is essentially just a function that accepts a node and returns an output. Handlers say which node types they are interested in:

registerHandler("color", node => {
  // Do something with the color node, like generating a color swatch.
});

Handlers return values

Each node in the AST has a given value. For example a color node has a node.value which is the actual color appearing in the declaration (e.g. red or #f06).

Without any special handlers, the system would simply generate those strings in the order they appear in the declaration.

So, really, the goal of a handler is to return something more interesting than just this default string. It may do so by either replacing this value, or adding something before, or after, or wrapping it.

As a result, a handler may return:

  • a simple string,
  • a DOM node (so you can wrap the value),
  • an array of these things (so you can generate something before or after the value),

Multiple handlers for the same type

It is of course possible to add many handlers on the same node type. We want to support the case where the rule-view comes built-in with a color swatch handler for color nodes and where an addon registers a new handler for color nodes that further augments this color swatch.

This is why handler functions don't only receive the node to be handled as an argument, but also the previously generated content.

Handlers run in the order they were added and therefore, the first handler will receive the raw node string as content, the second will receive whatever output the first handler has generated as content, and so on.

// The first, built-in, color-swatch generating handler.
registerHandler("color", (node, content) => {
  // Build a DOM node to display the color swatch.
  let el = doc.createElement("span");
  el.classList.add("color-swatch");
  el.setAttribute("style", `background:${node.value}`);
  
  // Return the color swatch before the existing content.
  return [el, content];
});

// The new color handler, which will run after the built-in one.
registerHandler("color", (node, content) => {
  // Here, content is the output of the previous handler.
  // Let's say we want to wrap it into a new DOM node that sets a few styles.

  // Let's first create the new DOM node.
  let el = doc.createElement("span");
  el.setAttribute("style", `color:${node.value};text-decoration:underline;`);
  
  // At this stage, content hasn't been rendered as a final output yet, we're still
  // just visiting the tree and aren't done, so content is an array. But we want
  // to wrap it, so force a final render, as a child of our new element.
  renderComponent(content, el);
  
  return el;
});

Note about React

The proposal makes very little assumptions about what is being returned by handlers so far. In fact, it would be pretty easy to render JSON, or strings instead of DOM nodes. It would be just as easy to return React components too. Because we're slowly moving to a React-based UI in DevTools, this proposal accounts for this. Handlers are pure-functions that can just return React components (or arrays of them, or components that wrap other React components).

Rendering the output

The rendering step comes after the parsing step and is when handlers are executed and where the final DOM output is built.

The tree is visited, node by node, recursively, and at each node, the list of handlers for that node type is retrieved, and they are all called in a sequence, with the next handlers consuming the output of the previous ones.

This step basically outputs an array containing DOM nodes and other arrays of arrays and DOM nodes, etc. that is then fed into the final rendering function that basically traverses the structure and appends the nodes to the rule-view.

Making changes to nodes

It's not nearly enough to render a static output, many of the handlers are going to want to change node values when certain user actions occur.

Let's take the color swatch example again. When the user clicks on the swatch to open the color picker, we want the changes there to be propagated through the system so the rule-view can know about it and re-render what needs to be re-rendered.

We don't want the color picker to be handled somewhere else like we do today (with a mix of the OutputParser generating the swatch, and the TextPropertyEditor later finding the swatch from the DOM, and the TooltipsOverlay then adding the tooltip to the swatch. We want the responsibility of displaying and altering a color to be at the same place.

This is why handlers also receive an emit function they can use to raise events when they want to change a node.

registerHandler("color", (node, content, emit) => {
  let el = doc.createElement("span");
  el.classList.add("color-swatch");
  el.setAttribute("style", `background:${node.value}`);
  
  // Listen to clicks on the swatch.
  el.addEventListener("click", () => {
    // Create/get and show the tooltip.
    let tooltip = getTooltip();
    tooltip.show(el);
    
    // Start listening to color changes in the tooltip.
    tooltip.on("changed", newColor => {
      let newValue = getDeclarationValueWithNodeChanged(node, newColor);
      emit("update-value", newValue);
    });
  });
  
  return [el, content];
});

The emit function is provided by the rule-view and used by handlers as an event emitter mechanism. The rule-view expects a few different events:

  • update-name to commit a change to the declaration name,
  • update-value to commit a change to the declaration value,
  • preview-value to preview a change to the declaration value.

Handlers are responsible for providing the final name or value for the declaration. The parser provides the helper getDeclarationValueWithNodeChanged to do this easily given a node and a new value for this node (this function uses the lexer token in the node to retrieve the start and end offset, and just assembles the new string).

From there, we let the existing rule-view update mechanism do its job. There is no changes there. The declaration will be sent to the server for preview, and the rule-view will be updated (which means the declaration will be parsed and rendered again, same as it is today).

TODO: in case of a preview only, we want the current handler to remain active instead of having the whole declaration re-rendered again. Therefore, each handler should keep a reference to their output DOM. Having handlers as React components would help here.

Examples of handlers

We've seen simplified code for a color handler up until now, but here are a few more examples.

Editing the name with the InplaceEditor

registerHandler("name", (node, content, updateValue) => {
  // Create a node for the name.
  let nameEl = createNode();

  // Use the InplaceEditor to allow edition of the nameEl's text content.
  editableField({
    element: nameEl,
    done: value => {
      updateValue(node, value);
    }
  });

  // Wrap the existing content in our element.
  renderComponent(content, nameEl);
  return nameEl;
});

Linkifying URLs and previewing images

function isUrlInBackgroundProperty(node) {
  // Retrieve the property name from the node. This is a helper function that
  // walks up the tree to find the declaration root, and then gets the name.
  let name = getPropertyName(node);
  let isInBackgroundProperty = name === "background" ||
                               name === "background-image";
  // Verify that the node is indeed an argument of a url function.
  let isInUrlFunction = node.parent.type === "argument" &&
                        node.parent.parent.value === "url";

  return isInBackgroundProperty && isInUrlFunction;
}

registerHandler("url", (node, content, updateValue) => {
  // Create a node for the url.
  let linkEl = createNode({
    nodeType: "a",
    attributes: {
      "href": node.value,
      "target": "_blank",
      "data-url": node.value
    }
  });

  // For background images, preview the image on hover.
  if (isUrlInBackgroundProperty(node)) {
    // Get the tooltip and toggle on hover of the url node.
    let tooltip = getTooltip();
    // ...
  }
  
  renderComponent(content, linkEl);
  return linkEl;
});

Simple formatting handlers

It turns out that you can even handle the formatting cases with this thing:

// A value node is in most cases, just the declaration value,
// but some properties accept multiple values, separated by comma (like multiple
// shadows), so a value node is one of these values.
// We want to separate them by comas.
registerHandler("value", (node, content) => {
  // The AST node has nextSibling and previousSibling properties we can use.
  return node.nextSibling ? [content, ", "] : content;
});

// Display function names and parens around the arguments.
registerHandler("function", (node, content) => {
  return [node.value, "(", content, ")"];
});

// Display comas between function arguments.
registerHandler("argument", (node, content) => {
  return node.nextSibling ? [content, ", "] : content;
});

Next steps

Once this is in place, and the rule-view has the same level of functionality than today, the next logical step is to support inline editors for things where we have tooltips today.

Inline editors are a little different in that they appear below the declaration in the rule-view, as an expanding section, so in a separate part of the DOM that handlers are not responsible for.

This means handlers will need to be able to emit events, a bit like they can call updateValue do have the rule-view do something, they will need to tell the rule-view that they are delegating edition of the value to an inline editor.

AST Node types

Container nodes

The following nodes represent the structure of the declaration.

declaration

This is always the root node of the AST. It has no parent node.

name

A declaration is a "name:value;" pair. There is only one name node in the AST and it is always the first child of the root node. It only contains the name of the CSS property as its node.value property.

values

Certain CSS properties can accept multiple values, like box-shadow which accepts multiple shadow definitions, separated by commas. For this reason, the "value" part in "name:value;" is always considered as an array of multiple values in the AST. Therefore, the values node is always the second child of the root node (it comes after the name node), and contains a list of values in its node.nodes property.

value

Child node of the values node. Any time the lexer encounters a comma that separate multiple property values, a new value node is inserted. This node contains no useful information, it just serves as a container for the actual value which is found in node.nodes.

function

Anything of the form foo() is considered a CSS function (including url()), so whenever one is encountered in a value, a function node is created. node.value contains the name of the function, and node.fullFunctionText contains the full text from the function name up until the closing brace. node.nodes contain a list of function arguments.

argument

Functions contain arguments. For example: rgb(255, 255, 255) has 3 arguments. Whenever a comma is encountered inside a function, it is considered as delimiting an argument. Just like value nodes, argument nodes don't contain useful information, they just serves as containers for the actual values which are found in node.nodes.

CSS types nodes

So far, the nodes described only serve as a way to represent the shape of a declaration. The nodes described below provide information about the actual CSS types present in declations, like colors or dimensions.

dimension

dimension nodes are numeric values with units. Like 12px or 100% for instance. A dimension node has 2 interesting properties: node.number which gives the actual numeric value, and node.unit which provides the unit for the value.

number

number nodes are similar to dimension nodes, except that they don't have a unit. A number node has the following interesting properties: node.number, node.hasSign and node.isInteger.

color

When things that look like colors are encountered in the declaration, they are checked for validity. So when a color node is present in the AST, it actually is a CSS color, not text that looks like a color name for example. color nodes can be either named colors, or hex colors. rgba, rgba, hsl, hsla are functions instead. color nodes have the actual color in node.value.

url

When a url function is encountered in the declaration, a url function node is created, that contains an argument node, that contains a url node. A url node only has one interesting property: node.value that contains the actual url. Note that the url is sanitized so that it is terminated correctly.

ident

ident nodes are created for any other piece of the value that isn't one of the CSS types above. So, for instance, the linear timing-function is treated as an ident node. The actual value is found in node.value.

symbol

Finally, for any symbol that hasn't been used as a delimiter for the name, values, value, function or argument nodes, a symbol node is created. That means that, even if there is a comma in between 2 box-shadow values, there will be no comma symbol node in the AST. This is unnecessary because 2 value nodes are created as children of the values node already. However, there are other symbols that don't separate containers and, for those, symbol nodes are created. Examples include the ! character before !important.

@tromey
Copy link

tromey commented May 27, 2016

I like this idea. It seems very clean, modular, and extensible.

I think there are a few corner cases to be addressed.

  • Currently output-parser.js doesn't display a swatch if a color is invalid for a given property, or inside the current function. These constraints would either have to be represented in the AST (by not making a color node in that case) or by the tree walker. It may help to try to more fully define the AST before beginning (and you'll want to do that anyway since the AST is a major part of the API contract). One question that comes to mind is how deep will the parsing go? For example you could go as far as a full CSS parser; or you could try to keep it "mostly lexical", just minorly augmenting the lexical structure with some known semantics ("this is validly a color").
  • I think it's worth considering how messy variables are. See https://bugzilla.mozilla.org/show_bug.cgi?id=1145527, both the patch, but also comment #9 and the other linked bugs. I don't want to overburden the initial design but at the same time it would be good to assure oneself that the resulting API won't prevent future improvements along those lines.
  • You'll have to decide what to do with invalid values. Can they be partially parsed? What would the AST look like? What about the termination cases handled by https://dxr.mozilla.org/mozilla-central/source/devtools/client/shared/output-parser.js#493?
  • Finally, one minor nit is that in an example you give a URL as function. It's not unreasonable to represent it this way in the AST, but currently, the CSS lexer treats URLs separately (since they are extra-special at the lexical level -- you cannot have a comment inside a url token). So, maybe the AST should just reflect that they are special as well.

@juliandescottes
Copy link

I like it, thanks!

Two comments:

  • should we notify the handlers when their node is being removed / replaced?
  • content can be a string, node or array. Would it make the handler's implementations easier if we only had arrays of Node/String elements ? With the current API, I can see handlers autoboxing content in an array before using it.

@captainbrosset
Copy link
Author

Thanks for the feedback so far.

Currently output-parser.js doesn't display a swatch if a color is invalid for a given property, or inside the current function. These constraints would either have to be represented in the AST (by not making a color node in that case) or by the tree walker.

Good point. I have changed my WIP patch to make sure no color AST node were created in those cases (doing the same thing that output-parser does now).

It may help to try to more fully define the AST before beginning (and you'll want to do that anyway since the AST is a major part of the API contract).

I will update the doc above to include all node types and properties for each.

One question that comes to mind is how deep will the parsing go? For example you could go as far as a full CSS parser; or you could try to keep it "mostly lexical", just minorly augmenting the lexical structure with some known semantics ("this is validly a color").

Right now in my WIP patch, it doesn't go very deep. It really just is concerned with what is required to make our current rule-view work.
It only knows about the name, the mutliple values, functions and their arguments, and then goes down to individual types, but that's very limited: number, dimension, percentage, color, ident only so far.
So that means that things like timing-functions are just parsed as idents, and it's then up to handlers to determine if these idents are timing-functions.
I probably is fine to add new types later, although when this starts being used by extensions, we'll need to make sure the API is stable.

I think it's worth considering how messy variables are. See https://bugzilla.mozilla.org/show_bug.cgi?id=1145527, both the patch, but also comment #9 and the other linked bugs. I don't want to overburden the initial design but at the same time it would be good to assure oneself that the resulting API won't prevent future improvements along those lines.

omg! css variables. I haven't really thought about those too much while working on this. Thanks for bringing this up. I'll need to spend more time on it.
I feel like the current proposal shouldn't prevent good support of css variables, but for sure this needs more thinking.

You'll have to decide what to do with invalid values. Can they be partially parsed? What would the AST look like? What about the termination cases handled by https://dxr.mozilla.org/mozilla-central/source/devtools/client/shared/output-parser.js#493?

I'll need to do some research about this. What types of invalid values can we have at this stage for instance (unclosed functions?). As for invalid URLs, I've reused the code in output-parser already, I think we should more or less be fine on this.

Finally, one minor nit is that in an example you give a URL as function. It's not unreasonable to represent it this way in the AST, but currently, the CSS lexer treats URLs separately (since they are extra-special at the lexical level -- you cannot have a comment inside a url token). So, maybe the AST should just reflect that they are special as well.

Yeah, I realized that URLs were treated differently when working on a WIP for this. But I decided to treat them as functions simply because then I can make all functions look and behave the same in the renderer. It's just a function node containing an argument node, so the handlers that are defined for these nodes will run for them too. And it also limits the number of different nodes that can exist in the AST.

should we notify the handlers when their node is being removed / replaced?

Good point! I haven't worried about that so far, but the handlers I've been working on are all adding event listeners and creating tooltips and such. So a destroy mechanism would be nice. I guess handlers could let the renderer know that they need to be destroyed and it would call their destroy function when required.
In a second phase, I'm hopping to react-ify all of this, and this would come for free.

content can be a string, node or array. Would it make the handler's implementations easier if we only had arrays of Node/String elements ? With the current API, I can see handlers autoboxing content in an array before using it.

Handlers shouldn't have to care about content. They either just return it as is if they don't want to modify the output, or return it as part of an array, or render it to a DOM node in order to wrap it into another DOM node. And I was thinking the renderer would always accept all 3 types: array, dom node or string.

@bgrins
Copy link

bgrins commented Jun 2, 2016

Thanks, this looks good. I was thinking about how this interacts with changes to the properties. Will a full parse happen when anything changes in the visible properties? That leads to some questions:

  1. In the "Making changes to nodes" section, what if the tooltip has been opened but then a different property changes and then registerHandler is called again? Would this system be not call the handler if that part of the AST wasn't changed?
  2. What if the property that changes is the one that already has a tooltip opened? Should the tooltip hide (and if so, how does it detect that the node has been destroyed?) Alternatively, what's the behavior to updateValue() if it's called with an invalid node that's no longer part of the tree?
  3. IIRC the output parser will ignore any changes that happened if an inplace editor is opened on any of the properties. If we went that route than maybe 1 and 2 aren't a problem, but how does one of these handlers signal that it's 'editing' to block updates from happening?

@oslego
Copy link

oslego commented Jun 11, 2016

This idea is very welcome.
I salute the intention to expose this to third-party extensions so that developers can build specialized inline editors.

I'll caveat anything I say below with the fact that I'm not familiar with the inner-workings of Firefox DevTools' Rule View. These are just things I foresee myself, as a third party developer, needing while developing inline editors.

  1. Destroy mechanism for handlers. This will be particularly useful for complex inline editors that spawn their own UI, like in-context editors displayed on top the inspected page. The handlers would benefit from receiving a termination event so they can teardown these related components.
  2. Avoid re-parsing / re-initializing handlers when updating own property value. This is probably well defined and understood. But I want to make it extra obvious that some inline editors will need to maintain state for the duration of the editing process. Constantly tearing down and rebuilding will interfere with the user's workflow.
    As a corollary to the above, it would probably be useful to re-trigger the inline editor if the property value changes as a result of some other action, like a media query, JavaScript interaction or manually editing style on the selected element in Elements panel. I understand this may be an edge case, but perhaps thinking of solving it may inform the API design for other side-effect interactions (CSS variables?).
  3. What are the options to allow handlers to mutate the AST / CSS Rule? Some inline editors will benefit from the ability to add or change CSS Declarations other than the current one.

Here are a few examples:

  • An editor for background-image may also want to set background-repeat, background-position, and so forth. This means mutating the existing CSS rule to add new declarations or overwriting existing ones.
  • An editor for display: flex may provide a shortcut to inject flex-direction, justify-content, `align-items, and so forth, in order to achieve and toggle between specific Flexbox layouts.
  • An editor that handles font-family may provide controls to toggle font-feature-settings. See http://clagnut.com/sandbox/css3/
  • Other non-related CSS properties may benefit from an editor that can toggle them in tandem to achieve specific effects, for example: mix-blend-mode and filter.

There is tremendous potential in this idea that can open up DevTools for specialized inline editors that can genuinely help web designers and developers.

Thank you for spending your energy working on it!

@captainbrosset
Copy link
Author

Thanks for the feedback all.
I've added a new file to this gist to start documenting the various AST nodes.

The big next steps will be managing the handlers' life cycle.
It's clear that handlers are more than just functions that generate a piece of the declaration output in the rule-view, they can, in some cases, be very complex pieces of UI. UIs that maintain a state, need to alter the output of the declaration, preview CSS changes, maybe add more declarations, and be alerted when they are destroyed/replaced.

In terms of changing values, I was imagining that handlers would raise events to the rule-view. These events would be update-value, update-name to change the value or the name in a way that the whole declaration then gets re-generated. Or preview-value to only preview a change without refreshing the whole declaration. This would be useful for things like the color picker that previews colors as you move your mouse but only commits the selected color on ENTER.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment