Rerender new icons #8
Conversation
adamwight
force-pushed
the
rerender-new-icons
branch
from
1e12fe1
to
70af46d
Compare
adamwight
force-pushed
the
rerender-new-icons
branch
from
70af46d
to
b7aa2bd
Compare
There was a problem hiding this comment.
I carefully reviewed every commit in this pull request and believe I can confirm everything. Note: There are two commits in here that are impossible to review via the GitHub UI. It just freezes. One way to avoid this in the future is to split this into two pull requests, one that updates all code and another that uploads all the new .png renders.
| return { | ||
| h: [hsl[0], hsl[0]], | ||
| s: [hsl[1], hsl[1]], | ||
| l: [hsl[2], hsl[2]], |
There was a problem hiding this comment.
I had a little fun playing around with this.
const hsl = blend.rgb2hsl( r, g, b ).map( ( v ) => [ v, v ] );
return { h: hsl[ 0 ], s: hsl[ 1 ], l: hsl[ 2 ] };Or even:
const hsl = blend.rgb2hsl( r, g, b );
return Object.fromEntries( hsl.map( ( v, i ) => [ 'hsl'[ i ], [ v, v ] ] ) );There was a problem hiding this comment.
Neither feels more readable, although the exercise is fun!
There was a problem hiding this comment.
Sure, this wasn't about readability. 😜
| * without the bad contrast math. | ||
| * | ||
| * @param {string} color 6-digit hex color | ||
| * @returns {object} HSL with [0-1] float components. Each term is a redundant |
There was a problem hiding this comment.
Type could be something like @return {Object.<string,number[]>} here, I think.
Also should use @return without the s.
There was a problem hiding this comment.
I don't think the more detailed type improves readability, though?
@returns seems to be the canonical name: https://jsdoc.app/tags-returns.html and it's only the wikimedia eslint plugin that enforces @return. On that topic, maybe we should turn on a linter, allow ES6, and rewrite var for example?
There was a problem hiding this comment.
Our canon is what the Wikimedia linter says. And we are in a Wikimedia codebase here, aren't we? But maybe this is an exception because it's a fork from something outside of the org. Let's not touch code style details for now.
There was a problem hiding this comment.
+1 that we've restyled repos in the past. Also +1 that it's a bigger job and best done by eslint --fix
The old (maki 0.5.0) icons were rendered with a large margin which is wasted space in a bitmap. Instead, this patch renders the full 15x15 (units) new icons in smaller boxes with no margin. Exact sizes were chosen such that new icons are roughly congruent to the old ones. There is a slight savings in the number of bits for each image. Includes a few more symbols.
adamwight
force-pushed
the
rerender-new-icons
branch
3 times, most recently
from
f0c3d93
to
8393ff4
Compare
This still builds static PNGs to serve, but the source step is done by a script. No longer paints a stroke around glyphs, to match the new Maki style guidelines. Removes the deprecated alphanum master doc.
…a threshold This makes the symbol visible on light backgrounds.
Was missing a color space conversion. The resulting formula has been checked against other references.
Calculate the luminance threshold dynamically, improving code maintainability in case we need to change the color again.
adamwight
force-pushed
the
rerender-new-icons
branch
from
8393ff4
to
648ae9b
Compare
Render fresh raster files for symbol and alphanumeric icons.