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

fix bug tick orient inverted #1348

Merged
merged 3 commits into from
May 11, 2016
Merged

fix bug tick orient inverted #1348

merged 3 commits into from
May 11, 2016

Conversation

YuhanLu
Copy link
Contributor

@YuhanLu YuhanLu commented May 10, 2016

Fix #1347
Bug is the default orient should be vertical, instead of horizontal.
Now this spec will give the following output correctly.

{
  "description": "Shows the relationship between horsepower and the numbver of cylinders using tick marks.",
  "data": {"url": "data/cars.json"},
  "mark": "tick",
  "encoding": {
    "x": {"field": "Horsepower", "type": "quantitative"},
    "y": {"field": "Cylinders", "type": "ordinal"}
  }
}

vega

@@ -42,7 +42,12 @@ describe('Mark: Bar', function() {
"y": {"bin": true, "type": "quantitative", "field": "IMDB_Rating"},
"x": {"scale": {"type": 'log'}, "type": "quantitative", "field": 'US_Gross', "aggregate": "mean"}
},
"data": {"url": 'data/movies.json'}
"data": {"url": 'data/movies.json'},
Copy link
Member

Choose a reason for hiding this comment

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

You're fixing default behavior in config.ts, but here modify the test to no longer test the default behavior. The correct fix is to fix the asserts below.

@kanitw
Copy link
Member

kanitw commented May 10, 2016

Can you provide a screenshot as well?

@YuhanLu
Copy link
Contributor Author

YuhanLu commented May 10, 2016

Fix the test.

assert.deepEqual(props.x2, {scale: 'x', value: 0});
});

it('should have no width', function(){
Copy link
Member

Choose a reason for hiding this comment

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

should have no height too? Why did you just remove it?

@YuhanLu
Copy link
Contributor Author

YuhanLu commented May 11, 2016

reset the default orient for tick along with tests

@kanitw kanitw merged commit 7d135c5 into master May 11, 2016
@kanitw kanitw deleted the zl/tick-orient branch May 11, 2016 03:57
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.

2 participants