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

fix: supply a valid axisconfig position default #32335

Merged
merged 1 commit into from Mar 2, 2019
Merged

fix: supply a valid axisconfig position default #32335

merged 1 commit into from Mar 2, 2019

Conversation

w33ble
Copy link
Contributor

@w33ble w33ble commented Mar 1, 2019

The previous default was an empty string, but empty strings are not valid in the expression. The code ends up using "left" or "top" (depending on the axis), so defaulting to either isn't really a change.

This fixes the following valid expression:

demodata | pointseries x=@timestamp y="mean(price)" | plot yaxis={axisConfig max=120}

Previously this would fail because the position default is invalid unless you specified a value.

screenshot 2019-03-01 12 11 47

Now it works as expected:

screenshot 2019-03-01 12 11 03

@w33ble
fix: supply a value position default
a23571b 
empty strings are invalid, and left is more or less the default anyway
@w33ble w33ble added review v7.0.0 Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas v8.0.0 v6.7.0 v7.2.0 v6.6.2 labels Mar 1, 2019
@w33ble w33ble requested a review from cqliu1 March 1, 2019 19:12
@w33ble w33ble requested a review from a team as a code owner March 1, 2019 19:12
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-canvas

@w33ble w33ble changed the title fix: supply an axisconfig position default fix: supply a valid axisconfig position default Mar 1, 2019
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

cqliu1
cqliu1 approved these changes Mar 2, 2019
View reviewed changes
Copy link
Contributor

@cqliu1 cqliu1 left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@w33ble w33ble merged commit 8440606 into elastic:master Mar 2, 2019
w33ble added a commit to w33ble/kibana that referenced this pull request Mar 2, 2019
@w33ble
fix: supply a valid axisconfig position default (elastic#32335)
01fc713 
The previous default was an empty string, but empty strings are not valid in the expression. The code ends up using "left" or "top" (depending on the axis), so defaulting to either isn't really a change.

This fixes the following valid expression:

```
demodata | pointseries x=@timestamp y="mean(price)" | plot yaxis={axisConfig max=120}
```

Previously this would fail because the `position` default is invalid unless you specified a value.

<img width="617" alt="screenshot 2019-03-01 12 11 47" src="https://user-images.githubusercontent.com/404731/53660475-33104a80-3c1b-11e9-9a4c-a93116598e28.png">

Now it works as expected:

<img width="536" alt="screenshot 2019-03-01 12 11 03" src="https://user-images.githubusercontent.com/404731/53660440-212ea780-3c1b-11e9-8f4f-7d83a1009fd0.png">
@w33ble w33ble mentioned this pull request Mar 2, 2019
Merged
w33ble added a commit to w33ble/kibana that referenced this pull request Mar 2, 2019
@w33ble
fix: supply a valid axisconfig position default (elastic#32335)
b344dd6 
The previous default was an empty string, but empty strings are not valid in the expression. The code ends up using "left" or "top" (depending on the axis), so defaulting to either isn't really a change.

This fixes the following valid expression:

```
demodata | pointseries x=@timestamp y="mean(price)" | plot yaxis={axisConfig max=120}
```

Previously this would fail because the `position` default is invalid unless you specified a value.

<img width="617" alt="screenshot 2019-03-01 12 11 47" src="https://user-images.githubusercontent.com/404731/53660475-33104a80-3c1b-11e9-9a4c-a93116598e28.png">

Now it works as expected:

<img width="536" alt="screenshot 2019-03-01 12 11 03" src="https://user-images.githubusercontent.com/404731/53660440-212ea780-3c1b-11e9-8f4f-7d83a1009fd0.png">
@w33ble w33ble mentioned this pull request Mar 2, 2019
Merged
w33ble added a commit to w33ble/kibana that referenced this pull request Mar 2, 2019
@w33ble
fix: supply a valid axisconfig position default (elastic#32335)
f6ecb60 
The previous default was an empty string, but empty strings are not valid in the expression. The code ends up using "left" or "top" (depending on the axis), so defaulting to either isn't really a change.

This fixes the following valid expression:

```
demodata | pointseries x=@timestamp y="mean(price)" | plot yaxis={axisConfig max=120}
```

Previously this would fail because the `position` default is invalid unless you specified a value.

<img width="617" alt="screenshot 2019-03-01 12 11 47" src="https://user-images.githubusercontent.com/404731/53660475-33104a80-3c1b-11e9-9a4c-a93116598e28.png">

Now it works as expected:

<img width="536" alt="screenshot 2019-03-01 12 11 03" src="https://user-images.githubusercontent.com/404731/53660440-212ea780-3c1b-11e9-8f4f-7d83a1009fd0.png">
This was referenced Mar 2, 2019
Merged
Merged
w33ble added a commit to w33ble/kibana that referenced this pull request Mar 2, 2019
@w33ble
fix: supply a valid axisconfig position default (elastic#32335)
bc83d36 
The previous default was an empty string, but empty strings are not valid in the expression. The code ends up using "left" or "top" (depending on the axis), so defaulting to either isn't really a change.

This fixes the following valid expression:

```
demodata | pointseries x=@timestamp y="mean(price)" | plot yaxis={axisConfig max=120}
```

Previously this would fail because the `position` default is invalid unless you specified a value.

<img width="617" alt="screenshot 2019-03-01 12 11 47" src="https://user-images.githubusercontent.com/404731/53660475-33104a80-3c1b-11e9-9a4c-a93116598e28.png">

Now it works as expected:

<img width="536" alt="screenshot 2019-03-01 12 11 03" src="https://user-images.githubusercontent.com/404731/53660440-212ea780-3c1b-11e9-8f4f-7d83a1009fd0.png">
w33ble added a commit that referenced this pull request Mar 5, 2019
@w33ble
fix: supply a valid axisconfig position default (#32335) (#32363)
314933f 
The previous default was an empty string, but empty strings are not valid in the expression. The code ends up using "left" or "top" (depending on the axis), so defaulting to either isn't really a change.

This fixes the following valid expression:

```
demodata | pointseries x=@timestamp y="mean(price)" | plot yaxis={axisConfig max=120}
```

Previously this would fail because the `position` default is invalid unless you specified a value.

<img width="617" alt="screenshot 2019-03-01 12 11 47" src="https://user-images.githubusercontent.com/404731/53660475-33104a80-3c1b-11e9-9a4c-a93116598e28.png">

Now it works as expected:

<img width="536" alt="screenshot 2019-03-01 12 11 03" src="https://user-images.githubusercontent.com/404731/53660440-212ea780-3c1b-11e9-8f4f-7d83a1009fd0.png">
w33ble added a commit that referenced this pull request Mar 5, 2019
@w33ble
fix: supply a valid axisconfig position default (#32335) (#32362)
62d2123 
The previous default was an empty string, but empty strings are not valid in the expression. The code ends up using "left" or "top" (depending on the axis), so defaulting to either isn't really a change.

This fixes the following valid expression:

```
demodata | pointseries x=@timestamp y="mean(price)" | plot yaxis={axisConfig max=120}
```

Previously this would fail because the `position` default is invalid unless you specified a value.

<img width="617" alt="screenshot 2019-03-01 12 11 47" src="https://user-images.githubusercontent.com/404731/53660475-33104a80-3c1b-11e9-9a4c-a93116598e28.png">

Now it works as expected:

<img width="536" alt="screenshot 2019-03-01 12 11 03" src="https://user-images.githubusercontent.com/404731/53660440-212ea780-3c1b-11e9-8f4f-7d83a1009fd0.png">
w33ble added a commit that referenced this pull request Mar 5, 2019
@w33ble
fix: supply a valid axisconfig position default (#32335) (#32361)
89969c5 
The previous default was an empty string, but empty strings are not valid in the expression. The code ends up using "left" or "top" (depending on the axis), so defaulting to either isn't really a change.

This fixes the following valid expression:

```
demodata | pointseries x=@timestamp y="mean(price)" | plot yaxis={axisConfig max=120}
```

Previously this would fail because the `position` default is invalid unless you specified a value.

<img width="617" alt="screenshot 2019-03-01 12 11 47" src="https://user-images.githubusercontent.com/404731/53660475-33104a80-3c1b-11e9-9a4c-a93116598e28.png">

Now it works as expected:

<img width="536" alt="screenshot 2019-03-01 12 11 03" src="https://user-images.githubusercontent.com/404731/53660440-212ea780-3c1b-11e9-8f4f-7d83a1009fd0.png">
w33ble added a commit that referenced this pull request Mar 5, 2019
@w33ble
fix: supply a valid axisconfig position default (#32335) (#32360)
e735314 
The previous default was an empty string, but empty strings are not valid in the expression. The code ends up using "left" or "top" (depending on the axis), so defaulting to either isn't really a change.

This fixes the following valid expression:

```
demodata | pointseries x=@timestamp y="mean(price)" | plot yaxis={axisConfig max=120}
```

Previously this would fail because the `position` default is invalid unless you specified a value.

<img width="617" alt="screenshot 2019-03-01 12 11 47" src="https://user-images.githubusercontent.com/404731/53660475-33104a80-3c1b-11e9-9a4c-a93116598e28.png">

Now it works as expected:

<img width="536" alt="screenshot 2019-03-01 12 11 03" src="https://user-images.githubusercontent.com/404731/53660440-212ea780-3c1b-11e9-8f4f-7d83a1009fd0.png">
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cqliu1 cqliu1 cqliu1 approved these changes

Assignees
No one assigned
Labels
review Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas v6.6.2 v6.7.0 v7.0.0 v7.2.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@w33ble @elasticmachine @cqliu1