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

[clay-charts] "xs" is missing from the state declaration of ChartBase #1064

Closed
kienD opened this issue Aug 1, 2018 · 1 comment
Closed
Labels
status: next-release Issues that will enter into the next release

Comments

@kienD
Copy link

kienD commented Aug 1, 2018

config.xs should be equal to state.xs but we do not declare "xs" in the state of ChartBase.

const config = {
color: state.colorFormatter,
colors: colors,
empty: state.emptyLabelText,
groups: state.groups,
hide: state.hide,
json: state.json,
keys: state.keys,
labels: state.labels,
mimeType: state.mimeType,
order: state.order,
rows: state.rows,
selection: state.selection,
type: state.type,
url: state.url,
x: state.x,
xFormat: state.xFormat,
xLocaltime: state.xLocaltime,
xSort: state.xSort,
xs: state.xs,

* @default undefined
* @instance
* @memberof ChartBase
* @type {?(boolean|undefined)}
*/
xLocaltime: Config.bool(),
/**
* Sets billboard's data.xSort config.
* @default undefined
* @instance
* @memberof ChartBase
* @type {?(Object|undefined)}
*/
xSort: Config.bool(),
/**
* Configuration for bb chart zoom capabilities.
* @default undefined
* @instance
* @memberof ChartBase
* @type {?(Object|undefined)}
*/
zoom: Config.shapeOf({
enabled: Config.bool().value(true),
rescale: Config.bool().value(false),
extent: Config.array(),
}),

@julien
Copy link
Contributor

julien commented Aug 1, 2018

Hi @kienD,

Thanks for reporting this!

I know the code might be a bit confusing but we actually map x to xs in ChartBase, which might have been a good idea some at some point.

See:

const PROP_NAME_MAP = {
axis: 'axes',
class: 'classes',
color: 'colors',
name: 'names',
regions: 'regions',
type: 'types',
x: 'xs',
};

And:

config[PROP_NAME_MAP[key]] =
config[PROP_NAME_MAP[key]] || {};
config[PROP_NAME_MAP[key]][id] = column[key];

But it appears that x and xs expect different types (String vs Object), so you're right, we need to add xs in ChartBase and stop "mapping" xs to x automatically.

Thanks

julien pushed a commit to julien/clay that referenced this issue Aug 2, 2018
carloslancha added a commit that referenced this issue Aug 8, 2018
Add xs property to CharBase's STATE | Fixes #1064
@carloslancha carloslancha added the status: next-release Issues that will enter into the next release label Aug 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: next-release Issues that will enter into the next release
Projects
None yet
Development

No branches or pull requests

3 participants