1364380 - Linear-gradient overlay in the page

Status: NEW

(DevTools :: Inspector, enhancement, P3)

Description

[Patrick Brosset <:pbro>]

Attachment: linear-gradient-WIP.diff

When working on linear-gradients in the rule-view, it would be good if the gradient line, box, and stops were displayed in the page, so it would be easier to understand how the gradient gets rendered.

I had made a gradient visualizer a long time ago (http://patrickbrosset.com/lab/2015-03-11-css-linear-gradient-overlay.html), and had worked on a patch to include this into DevTools.
Of course, I never had the time to finalize this, so the patch just sat in my repo for the longest time, now very outdated.

I'm still attaching it here because there is code that can still be salvaged I believe. And I'd really like to see this land at some stage.

Comment 1

[Sebastian Zartner [:sebo]]

Really great work! It would be fantastic to see that work in the DevTools!

I see three solutions how this could be implemented:
1. Like the grid highlighter
There's a badge besides the gradient (in a list, one besides each gradient) allowing to toggle the highlighter.
2. Like the transform highlighter
The highlighter would be toggled by hovering the gradient (in a list, each gradient individually).
3. Make it an in-page editor
Like the first option, though allowing to edit the gradient within the page, i.e. drag the color stops and hints and change the angle of the gradient.

Notes regarding the visualizer:
It doesn't support color hints. And it should be adjusted to also work with radial gradients and repeating gradients.

Sebastian

Comment 2

[Patrick Brosset <:pbro>]

DevTools bug triage (filter on CLIMBING SHOES).

Comment 3

[Patrick Brosset <:pbro>]

1 year later, I got interested in this again, so I resurrected the patch and made it work in today's devtools codebase.
I'll attach it next.
It's just the highlighter part though, so you have to start it via scratchpad for now.
Notable: I added support for background size and position.
However the harder things like supporting css transform are still not in place. Also there's a problem with how the underlying canvas is positioned/sized as it does not always cover the background area.

Comment 4

[Patrick Brosset <:pbro>]

Attachment: Bug 1364380 - New Linear-gradient highlighter

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

Comment 5

[Patrick Brosset <:pbro>]

Comment on attachment 8983041 [details]
Bug 1364380 - New Linear-gradient highlighter

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

Comment 6

[Code Review Bot [:reviewbot]]

Comment on attachment 8983041 [details]
Bug 1364380 - New Linear-gradient highlighter

https://reviewboard.mozilla.org/r/248906/#review255058


Code analysis found 11 defects in this patch:
 - 11 defects found by mozlint

You can run this analysis locally with:
 - `./mach lint path/to/file` (JS/Python)


If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx


::: devtools/shared/css/parsing-utils.js:1396
(Diff revision 2)
> +  // Separate into multiple background functions.
> +  const backgrounds = [];
> +  const stack = [];
> +
> +  while (true) {
> +    let token = lexer.nextToken();

Error: 'token' is never reassigned. use 'const' instead. [eslint: prefer-const]

::: devtools/shared/css/parsing-utils.js:1431
(Diff revision 2)
> +      }
> +
> +      stack[stack.length - 1].children.push(token);
> +    }
> +
> +  }

Error: Block must not be padded by blank lines. [eslint: padded-blocks]

::: devtools/shared/css/parsing-utils.js:1455
(Diff revision 2)
> +}
> +
> +function resolveRgbColor(rgbToken) {
> +  let color = rgbToken.text + "(";
> +
> +  for (let token of rgbToken.children) {

Error: 'token' is never reassigned. use 'const' instead. [eslint: prefer-const]

::: devtools/shared/css/parsing-utils.js:1498
(Diff revision 2)
> +
> +function parseGradient(computedBackgroundImage, index, bounds) {
> +  const backgroundImageTokens = getGradientTokenTree(computedBackgroundImage);
> +  const tokenTree = backgroundImageTokens[index];
> +
> +  if (GRADIENT_TYPES.indexOf(tokenTree.text) === -1) {

Error: Use .includes instead of .indexof [eslint: mozilla/use-includes-instead-of-indexOf]

::: devtools/shared/css/parsing-utils.js:1502
(Diff revision 2)
> +
> +  if (GRADIENT_TYPES.indexOf(tokenTree.text) === -1) {
> +    throw new Error(tokenTree.text + " isn't a valid gradient type");
> +  }
> +
> +  let data = {

Error: 'data' is never reassigned. use 'const' instead. [eslint: prefer-const]

::: devtools/shared/css/parsing-utils.js:1510
(Diff revision 2)
> +    stops: []
> +  };
> +
> +  // Go over all tokens in this gradient and collect interesting ones (angle, if any,
> +  // colors, positions).
> +  let angleKeyword = [];

Error: 'anglekeyword' is never reassigned. use 'const' instead. [eslint: prefer-const]

::: devtools/shared/css/parsing-utils.js:1512
(Diff revision 2)
> +
> +  // Go over all tokens in this gradient and collect interesting ones (angle, if any,
> +  // colors, positions).
> +  let angleKeyword = [];
> +
> +  for (let token of tokenTree.children) {

Error: 'token' is never reassigned. use 'const' instead. [eslint: prefer-const]

::: devtools/shared/css/parsing-utils.js:1513
(Diff revision 2)
> +  // Go over all tokens in this gradient and collect interesting ones (angle, if any,
> +  // colors, positions).
> +  let angleKeyword = [];
> +
> +  for (let token of tokenTree.children) {
> +    let {tokenType: type, text} = token;

Error: 'type' is never reassigned. use 'const' instead. [eslint: prefer-const]

::: devtools/shared/css/parsing-utils.js:1513
(Diff revision 2)
> +  // Go over all tokens in this gradient and collect interesting ones (angle, if any,
> +  // colors, positions).
> +  let angleKeyword = [];
> +
> +  for (let token of tokenTree.children) {
> +    let {tokenType: type, text} = token;

Error: 'text' is never reassigned. use 'const' instead. [eslint: prefer-const]

::: devtools/shared/css/parsing-utils.js:1530
(Diff revision 2)
> +      // Color stop position.
> +      data.stops[data.stops.length - 1].position = type === "percentage"
> +                                                   ? token.number * 100 + "%"
> +                                                   : token.number + text;
> +    } else if (type === "ident" &&
> +               SPECIAL_COLORS.indexOf(text.toLowerCase()) !== -1) {

Error: Use .includes instead of .indexof [eslint: mozilla/use-includes-instead-of-indexOf]

::: devtools/shared/css/parsing-utils.js:1551
(Diff revision 2)
> +
> +  // Calculate the gradient line based on the angle.
> +  data.gradientLine = getGradientLine(data.angle, bounds);
> +
> +  // Make sure all stops are in %.
> +  for (let stop of data.stops) {

Error: 'stop' is never reassigned. use 'const' instead. [eslint: prefer-const]

Comment 7

[Code Review Bot [:reviewbot]]

Comment on attachment 8983041 [details]
Bug 1364380 - New Linear-gradient highlighter

https://reviewboard.mozilla.org/r/248906/#review255062


Code analysis found 3 defects in this patch:
 - 3 defects found by mozlint

You can run this analysis locally with:
 - `./mach lint path/to/file` (JS/Python)


If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx


::: devtools/shared/css/parsing-utils.js:1431
(Diff revision 1)
> +      }
> +
> +      stack[stack.length - 1].children.push(token);
> +    }
> +
> +  }

Error: Block must not be padded by blank lines. [eslint: padded-blocks]

::: devtools/shared/css/parsing-utils.js:1498
(Diff revision 1)
> +
> +function parseGradient(computedBackgroundImage, index, bounds) {
> +  const backgroundImageTokens = getGradientTokenTree(computedBackgroundImage);
> +  const tokenTree = backgroundImageTokens[index];
> +
> +  if (GRADIENT_TYPES.indexOf(tokenTree.text) === -1) {

Error: Use .includes instead of .indexof [eslint: mozilla/use-includes-instead-of-indexOf]

::: devtools/shared/css/parsing-utils.js:1530
(Diff revision 1)
> +      // Color stop position.
> +      data.stops[data.stops.length - 1].position = type === "percentage"
> +                                                   ? token.number * 100 + "%"
> +                                                   : token.number + text;
> +    } else if (type === "ident" &&
> +               SPECIAL_COLORS.indexOf(text.toLowerCase()) !== -1) {

Error: Use .includes instead of .indexof [eslint: mozilla/use-includes-instead-of-indexOf]

Comment 8

[Patrick Brosset <:pbro>]

Attachment: Bug 1364380 - Gradient swatch in the rule-view to display the highlighter WIP

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

Comment 9

[Patrick Brosset <:pbro>]

Comment on attachment 8983041 [details]
Bug 1364380 - New Linear-gradient highlighter

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

Comment 10

[Code Review Bot [:reviewbot]]

Comment on attachment 8983041 [details]
Bug 1364380 - New Linear-gradient highlighter

https://reviewboard.mozilla.org/r/248906/#review267544


Code analysis found 11 defects in this patch:
 - 11 defects found by mozlint

You can run this analysis locally with:
 - `./mach lint path/to/file` (JS/Python)


If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx


::: devtools/shared/css/parsing-utils.js:1400
(Diff revision 3)
> +  // Separate into multiple background functions.
> +  const backgrounds = [];
> +  const stack = [];
> +
> +  while (true) {
> +    let token = lexer.nextToken();

Error: 'token' is never reassigned. Use 'const' instead. [eslint: prefer-const]

::: devtools/shared/css/parsing-utils.js:1439
(Diff revision 3)
> +      }
> +
> +      stack[stack.length - 1].children.push(token);
> +    }
> +
> +  }

Error: Block must not be padded by blank lines. [eslint: padded-blocks]

::: devtools/shared/css/parsing-utils.js:1463
(Diff revision 3)
> +}
> +
> +function resolveRgbColor(rgbToken) {
> +  let color = rgbToken.text + "(";
> +
> +  for (let token of rgbToken.children) {

Error: 'token' is never reassigned. Use 'const' instead. [eslint: prefer-const]

::: devtools/shared/css/parsing-utils.js:1510
(Diff revision 3)
> +
> +  if (!tokenTree) {
> +    throw new Error("Could not parse the gradient");
> +  }
> +
> +  if (GRADIENT_TYPES.indexOf(tokenTree.text) === -1) {

Error: Use .includes instead of .indexOf [eslint: mozilla/use-includes-instead-of-indexOf]

::: devtools/shared/css/parsing-utils.js:1514
(Diff revision 3)
> +
> +  if (GRADIENT_TYPES.indexOf(tokenTree.text) === -1) {
> +    throw new Error(tokenTree.text + " isn't a valid gradient type");
> +  }
> +
> +  let data = {

Error: 'data' is never reassigned. Use 'const' instead. [eslint: prefer-const]

::: devtools/shared/css/parsing-utils.js:1522
(Diff revision 3)
> +    stops: []
> +  };
> +
> +  // Go over all tokens in this gradient and collect interesting ones (angle, if any,
> +  // colors, positions).
> +  let angleKeyword = [];

Error: 'angleKeyword' is never reassigned. Use 'const' instead. [eslint: prefer-const]

::: devtools/shared/css/parsing-utils.js:1524
(Diff revision 3)
> +
> +  // Go over all tokens in this gradient and collect interesting ones (angle, if any,
> +  // colors, positions).
> +  let angleKeyword = [];
> +
> +  for (let token of tokenTree.children) {

Error: 'token' is never reassigned. Use 'const' instead. [eslint: prefer-const]

::: devtools/shared/css/parsing-utils.js:1525
(Diff revision 3)
> +  // Go over all tokens in this gradient and collect interesting ones (angle, if any,
> +  // colors, positions).
> +  let angleKeyword = [];
> +
> +  for (let token of tokenTree.children) {
> +    let {tokenType: type, text} = token;

Error: 'type' is never reassigned. Use 'const' instead. [eslint: prefer-const]

::: devtools/shared/css/parsing-utils.js:1525
(Diff revision 3)
> +  // Go over all tokens in this gradient and collect interesting ones (angle, if any,
> +  // colors, positions).
> +  let angleKeyword = [];
> +
> +  for (let token of tokenTree.children) {
> +    let {tokenType: type, text} = token;

Error: 'text' is never reassigned. Use 'const' instead. [eslint: prefer-const]

::: devtools/shared/css/parsing-utils.js:1542
(Diff revision 3)
> +      // Color stop position.
> +      data.stops[data.stops.length - 1].position = type === "percentage"
> +                                                   ? token.number * 100 + "%"
> +                                                   : token.number + text;
> +    } else if (type === "ident" &&
> +               SPECIAL_COLORS.indexOf(text.toLowerCase()) !== -1) {

Error: Use .includes instead of .indexOf [eslint: mozilla/use-includes-instead-of-indexOf]

::: devtools/shared/css/parsing-utils.js:1563
(Diff revision 3)
> +
> +  // Calculate the gradient line based on the angle.
> +  data.gradientLine = getGradientLine(data.angle, bounds);
> +
> +  // Make sure all stops are in %.
> +  for (let stop of data.stops) {

Error: 'stop' is never reassigned. Use 'const' instead. [eslint: prefer-const]

Comment 11

[Code Review Bot [:reviewbot]]

Comment on attachment 8996329 [details]
Bug 1364380 - Gradient swatch in the rule-view to display the highlighter WIP

https://reviewboard.mozilla.org/r/260470/#review267548


Code analysis found 12 defects in this patch:
 - 12 defects found by mozlint

You can run this analysis locally with:
 - `./mach lint path/to/file` (JS/Python)


If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx


::: devtools/client/inspector/shared/highlighters-overlay.js:226
(Diff revision 1)
> +      return;
> +    }
> +
> +    const isShown = await gradientHighlighter.show(node, options);
> +    if (!isShown) {
> +      return;

Error: Unnecessary return statement. [eslint: no-useless-return]