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.

Sign up for GitHub

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

Jump to bottom

feat: Improve color scheme type #9015

Merged
merged 8 commits into from
Jul 19, 2023
Merged

feat: Improve color scheme type #9015

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
6a61ebb
types: add vega scheme type to vl scheme type
mcnuttandrew Jul 18, 2023
a9b93a2
chore: update schema [CI]
Jul 18, 2023
fa6abce
chore: update examples [CI]
Jul 18, 2023
16b2ff4
undo removed accidentally type
mcnuttandrew Jul 18, 2023
ed54fc3
merge
mcnuttandrew Jul 18, 2023
1dfd3b7
Why did autoformat change so much code, a mystery
mcnuttandrew Jul 18, 2023
ae87cb4
respond to comments
mcnuttandrew Jul 19, 2023
ab33d2a
update tests, undo accidental img updates
mcnuttandrew Jul 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions build/vega-lite-schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21914,6 +21914,9 @@
{
"type": "string"
},
{
"$ref": "#/definitions/ColorScheme"
},
{
"$ref": "#/definitions/SchemeParams"
},
Expand Down Expand Up @@ -22468,8 +22471,15 @@
"type": "array"
},
"name": {
"description": "A color scheme name for ordinal scales (e.g., `\"category10\"` or `\"blues\"`).\n\nFor the full list of supported schemes, please refer to the [Vega Scheme](https://vega.github.io/vega/docs/schemes/#reference) reference.",
"type": "string"
"anyOf": [
{
"type": "string"
},
{
"$ref": "#/definitions/ColorScheme"
}
],
"description": "A color scheme name for ordinal scales (e.g., `\"category10\"` or `\"blues\"`).\n\nFor the full list of supported schemes, please refer to the [Vega Scheme](https://vega.github.io/vega/docs/schemes/#reference) reference."
}
},
"required": [
Expand Down
Binary file modified examples/compiled/interactive_point_domainRaw_binding.png
View file
Open in desktop
domoritz marked this conversation as resolved.
Show resolved Hide resolved
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion examples/compiled/interactive_point_domainRaw_binding.svg
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
1 change: 1 addition & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@
"vega-datasets": "^2.7.0",
"vega-embed": "^6.22.1",
"vega-tooltip": "^0.32.0",
"vega-typings": "^0.24.1",
Copy link
Member

Choose a reason for hiding this comment

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

Since we already depend on Vega, I don’t think we need this. It just makes updates more annoying.

"yaml-front-matter": "^4.1.1"
},
"dependencies": {
Expand Down
5 changes: 3 additions & 2 deletions src/scale.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
TimeIntervalStep
} from 'vega';
import {isString} from 'vega-util';
import type {ColorScheme} from 'vega-typings';
import * as CHANNEL from './channel';
import {Channel, isColorChannel} from './channel';
import {DateTime} from './datetime';
Expand Down Expand Up @@ -458,7 +459,7 @@ export interface SchemeParams {
*
* For the full list of supported schemes, please refer to the [Vega Scheme](https://vega.github.io/vega/docs/schemes/#reference) reference.
*/
name: string | SignalRef;
name: string | ColorScheme | SignalRef;
Copy link
Member

Choose a reason for hiding this comment

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

This is great. Should we even allow string?

Copy link
Contributor Author

@mcnuttandrew mcnuttandrew Jul 19, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Good point! I went with a more permissive type because I wasn't sure what the down stream deps were from this type, but if this seems fine to you then a more assertive type is great!


/**
* The extent of the color range to use. For example `[0.2, 1]` will rescale the color scheme such that color values in the range _[0, 0.2)_ are excluded from the scheme.
Expand Down Expand Up @@ -610,7 +611,7 @@ export interface Scale<ES extends ExprRef | SignalRef = ExprRef | SignalRef> {
*
* For the full list of supported schemes, please refer to the [Vega Scheme](https://vega.github.io/vega/docs/schemes/#reference) reference.
*/
scheme?: string | SchemeParams | ES;
scheme?: string | ColorScheme | SchemeParams | ES;

/**
* The alignment of the steps within the scale range.
Expand Down
2 changes: 1 addition & 1 deletion yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9015,7 +9015,7 @@ vega-transforms@~4.10.2:
vega-time "^2.1.1"
vega-util "^1.17.1"

vega-typings@~0.24.0:
vega-typings@^0.24.1, vega-typings@~0.24.0:
version "0.24.1"
resolved "https://registry.yarnpkg.com/vega-typings/-/vega-typings-0.24.1.tgz#7e1e87557a9bf09e934687197c5e964bd9b416ca"
integrity sha512-WNw6tDxwMsynQ9osJb3RZi3g8GZruxVgXfe8N7nbqvNOgDQkUuVjqTZiwGg5kqjmLqx09lRRlskgp/ov7lEGeg==
Expand Down