1459898 - Font editor: implement conversion between unit types

Status: RESOLVED FIXED

(DevTools :: Inspector, enhancement, P3)

Description

[Razvan Caliman [:rcaliman]]

Allow the user to toggle between unit types in font property inputs and have the values adapt proportionally to the new unit.

Comment 1

[Razvan Caliman [:rcaliman]]

Attachment: Bug 1459898 - (Part 1) Convert between unit types for font-size in font editor.

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

Comment 2

[Razvan Caliman [:rcaliman]]

Attachment: Bug 1459898 - (Part 2) Add test for font-size unit conversion.

Move getPropertyValue() to devtools/client/inspector/fonts/test/head.js

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

Comment 3

[Razvan Caliman [:rcaliman]]

Comment on attachment 8990737 [details]
Bug 1459898 - (Part 1) Convert between unit types for font-size in font editor.

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

Comment 4

[Razvan Caliman [:rcaliman]]

Comment on attachment 8990737 [details]
Bug 1459898 - (Part 1) Convert between unit types for font-size in font editor.

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

Comment 5

[Razvan Caliman [:rcaliman]]

Comment on attachment 8990769 [details]
Bug 1459898 - (Part 2) Add test for font-size unit conversion.

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

Comment 6

[Razvan Caliman [:rcaliman]]

Comment on attachment 8990769 [details]
Bug 1459898 - (Part 2) Add test for font-size unit conversion.

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

Comment 7

[Gabriel Luong [:gl] (ΦωΦ)]

Comment on attachment 8990737 [details]
Bug 1459898 - (Part 1) Convert between unit types for font-size in font editor.

https://reviewboard.mozilla.org/r/255804/#review263108

::: devtools/client/inspector/fonts/fonts.js:144
(Diff revision 3)
>    /**
> +   * Convert a value for font-size between two CSS unit types.
> +   * Conversion is done via pixels. If neither of the two given unit types is "px",
> +   * recursively get the value in pixels, then convert that result to the desired unit.
> +   *
> +   * @param {Number} value

I think we are missing an extra space after the @param to get it to align with the @return that we have done in this file.

::: devtools/client/inspector/fonts/fonts.js:153
(Diff revision 3)
> +   * @param {String} toUnit
> +   *        CSS unit to convert to.
> +   * @return {Number}
> +   *         Converted numeric value.
> +   */
> +  async convert(value, fromUnit, toUnit) {

s/convert/convertUnits

Comment 8

[Gabriel Luong [:gl] (ΦωΦ)]

Comment on attachment 8990769 [details]
Bug 1459898 - (Part 2) Add test for font-size unit conversion.

https://reviewboard.mozilla.org/r/255820/#review263110

::: devtools/client/inspector/fonts/test/head.js:224
(Diff revision 3)
> +}
> +
> +/**
> + * Wait for a predicate to return a result.
> + *
> + * @param {Function} condition

Add an extra space after @param for the proper alignment

Comment 9

[Razvan Caliman [:rcaliman]]

Comment on attachment 8990737 [details]
Bug 1459898 - (Part 1) Convert between unit types for font-size in font editor.

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

Comment 10

[Razvan Caliman [:rcaliman]]

Comment on attachment 8990769 [details]
Bug 1459898 - (Part 2) Add test for font-size unit conversion.

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

Comment 11

[Razvan Caliman [:rcaliman]]

Comment on attachment 8990769 [details]
Bug 1459898 - (Part 2) Add test for font-size unit conversion.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/255820/diff/4-5/

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d6409658af22
(Part 1) Convert between unit types for font-size in font editor. r=gl
https://hg.mozilla.org/integration/autoland/rev/bbbbc8a982fe
(Part 2) Add test for font-size unit conversion. r=gl

Comment 13

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/d6409658af22
https://hg.mozilla.org/mozilla-central/rev/bbbbc8a982fe