githubocto / flat-ui

MIT License
373 stars 23 forks source link

Fix schema `color` bug #22

Closed nikodraca closed 2 years ago

nikodraca commented 2 years ago

The logic for inferring colors in the schema was setting any string field as a color. This is because the d3.rgb still returns an object even if the color isn't valid:

> const {rgb} = require('d3');
> rgb("not-a-color")
Rgb { r: NaN, g: NaN, b: NaN, opacity: NaN }

This checks to see if the return values are non-null.

Closes #21

Wattenberger commented 2 years ago

thanks for the PR @nikodraca! this should have been handled in commit https://github.com/githubocto/flat-ui/commit/00df00c04bfa9eac55ca91ce46bbc2b0ebbc37e6

I think destructuring color would lead to errors, since rgb() can return null, eg for an empty string:

image

testing in this Observable notebook

definitely let me know if the current check return !!color && !Number.isNaN(color.r) is missing any use cases, and thanks for taking the time here!

nikodraca commented 2 years ago

Ah my bad! I didn't see this was updated in the mean time.

Wattenberger commented 2 years ago

haha I snuck it past you! thanks for the time and thought, though!