Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add color to preset #183

Merged
merged 6 commits into from
May 15, 2024
Merged

feat: add color to preset #183

merged 6 commits into from
May 15, 2024

Conversation

tomasciccola
Copy link
Contributor

@tomasciccola tomasciccola commented May 14, 2024

should fix #182

Validation is done at the JSONSchema level by using a regex pattern in the schema itself (that only allows colors in format #rrggbb) and before encoding by using the validate-color lib.

Looking at the default config, I see there are some presets that have no color. How should we handle that? A possibility is to add a default color on the config-builder (I don't think that should live here but maybe it should??)

@tomasciccola tomasciccola self-assigned this May 14, 2024
@tomasciccola tomasciccola requested a review from EvanHahn May 14, 2024 18:28
@tomasciccola tomasciccola marked this pull request as ready for review May 14, 2024 18:54
Copy link
Contributor

@EvanHahn EvanHahn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wondering: should we validate when encoding that a color value is a 24-bit color string? for now, validation is done on JSONSchema level by adding a pattern with a regex in the schema itself. This means that passing an invalid string would throw. I wonder how we should handle that?
just let it throw?
Should we have a default color as a fallback? Should this default color be handled here? It could also live on the settings-builder (basically if no color or invalid color lives on a preset, then add the default one...)

IMO, we should let it throw here because the color is invalid. The settings builder seems like a great place to use a fallback.

schema/preset/v2.json Outdated Show resolved Hide resolved
src/lib/encode-conversions.ts Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add "color" field to Presets
2 participants