Add width/height as a top level specification for CQL queries. #415
Conversation
| * Thus the `encoding` object in Vega-Lite is flatten as the `encodings` array in CompassQL. | ||
| */ | ||
| encodings: EncodingQuery[]; | ||
|
|
||
| /** |
There was a problem hiding this comment.
Remove the JSDoc comment or change it to normal comment (// width and height//?
Note that this comment would be only applied to height when you hover over width variable. So if we have descriptive comment, better make them separate for width / height. However, if the width property's JSDocs is simply "width", there is no point adding it.
There was a problem hiding this comment.
I think a good JSDocs note for these two variables are that width cannot be wildcard (yet). Same for height.
(I don't think we need to implement them as wildcard yet as there is no required use case.)
|
Still need to do this?
And maybe test if this really works in the editor, if it works, capture a screenshot then I think we should be good to go. |
|
Oh yes, I missed that comment, sorry. Should be done! Here's a screenshot. |
src/query/shorthand.ts
Outdated
| @@ -134,6 +134,13 @@ export function spec(specQ: SpecQuery, | |||
| } | |||
| } | |||
|
|
|||
| if (!!specQ.width) { | |||
There was a problem hiding this comment.
I think you have to take include into account here too.
(I also notice that we forgot to do the same for transform above -- please help fix that too -- maybe in a separate PR.)
src/query/spec.ts
Outdated
| * Thus the `encoding` object in Vega-Lite is flatten as the `encodings` array in CompassQL. | ||
| */ | ||
| encodings: EncodingQuery[]; | ||
|
|
||
| /** | ||
| * The width of the resulting encodings, does not support wildcards. |
There was a problem hiding this comment.
I think splitting the sentence makes it a bit easier to read :)
The width of the resulting encodings.
__Note:__ This property does not support wildcards.
|
One thing that I start wondering is whether we should add other top-level properties of to CompassQL. (They are probably less important than adding |
|
Don't forget to take care of "include" :) |
…rthand accordingly
|
Refactored for include! |
|
Okay sounds, good, would we want to include mark also as a 'Top Level Property'? |
Um let's say no -- because mark can have nested properties and top-level properties in VL is applicable for single view and multi view (while mark is only for single view) |
|
I think your latest commit which says "add test" haven't really add test. lol |
Width height can be specified to control the dimensions of the output vega-lite specs that come out of
recommend. See issue #332