Fix hidden ticks when using log scale #21507
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
markov00
force-pushed
the
fix/regression-21499
branch
from
c74e798
to
02b5ba8
Compare
The vertical axis scale has an inverted range (max, min) and we need to compute the absolute scale width instead.
markov00
force-pushed
the
fix/regression-21499
branch
from
02b5ba8
to
e82ce22
Compare
markov00
added
Feature:Visualizations
💚 Build Succeeded |
timroes
added
bug
💔 Build Failed |
💔 Build Failed |
markov00
force-pushed
the
fix/regression-21499
branch
from
3c64636
to
59a55f7
Compare
💚 Build Succeeded |
timroes
reviewed
Aug 2, 2018
| await PageObjects.visualize.clickYAxisAdvancedOptions(axisId); | ||
| await PageObjects.visualize.changeYAxisFilterLabelsCheckbox(axisId, false); | ||
| await PageObjects.visualize.clickGo(); | ||
| await PageObjects.header.waitUntilLoadingHasFinished(); |
There was a problem hiding this comment.
You can remove the waitUntilLoadingHasFInished behind all clickGo, since the clickGo function should (and does) wait for all rendering that is needed before finishing.
timroes
approved these changes
Aug 2, 2018
ppisljar
reviewed
Aug 2, 2018
| @@ -153,5 +155,85 @@ export default function ({ getService, getPageObjects }) { | |||
| const sideEditorExists = await PageObjects.visualize.getSideEditorExists(); | |||
| expect(sideEditorExists).to.be(false); | |||
| }); | |||
|
|
|||
| describe('switch between Y axis scale types', () => { | |||
| before(initAreaChart); | |||
There was a problem hiding this comment.
why do we initAreaChart twice ? (before describe and before it in this describe)
There was a problem hiding this comment.
could we make all this tests go on without re-initing the charts ? this way the tests will run much faster. if one fails all the subsequent ones will fail as well, but i think that's not the problem as the first failing one already indicated issues with axis scaling.
There was a problem hiding this comment.
I've just followed what was already there in functional tests on vertical_bar_chart test.
I will remove all additional before calls and check if this still works correctly.
There was a problem hiding this comment.
@ppisljar I've take a better look at other tests code: I think there are two way to manage this:
- reintialize the chart (having a clean state on the test)
- go back to a clean state on the test like (reopen the saved chart or similar) because in the case of the areachart for example, the last test opens the visualization in embed mode. We need to go back to the editor before proceeding.
Reinitialization seems to be the clean way to work (and you are not dependent on the side effects produced by other tests (this will also allows you to avoid breaking tests when adding new tests without taking care where to put your test.
What do you think?
ppisljar
requested changes
Aug 2, 2018
💚 Build Succeeded |
markov00
added a commit
to markov00/kibana
that referenced
this pull request
Aug 3, 2018
* Add support for vertical axis The vertical axis scale has an inverted range (max, min) and we need to compute the absolute scale width instead. * Add functional test for Y axis scale type switching * Remove unnecessary waitUntilLoadingHasFinished
markov00
added a commit
to markov00/kibana
that referenced
this pull request
Aug 3, 2018
* Add support for vertical axis The vertical axis scale has an inverted range (max, min) and we need to compute the absolute scale width instead. * Add functional test for Y axis scale type switching * Remove unnecessary waitUntilLoadingHasFinished
markov00
added a commit
that referenced
this pull request
Aug 3, 2018
…k tests because of possible flakyness (#21641) (#21647) * Fix hidden ticks when using log scale (#21507) * Add support for vertical axis The vertical axis scale has an inverted range (max, min) and we need to compute the absolute scale width instead. * Add functional test for Y axis scale type switching * Remove unnecessary waitUntilLoadingHasFinished * Skip scale tick tests because of possible flakyness (#21641)
markov00
added a commit
that referenced
this pull request
Aug 3, 2018
…k tests because of possible flakyness (#21641) (#21648) * Fix hidden ticks when using log scale (#21507) * Add support for vertical axis The vertical axis scale has an inverted range (max, min) and we need to compute the absolute scale width instead. * Add functional test for Y axis scale type switching * Remove unnecessary waitUntilLoadingHasFinished * Skip scale tick tests because of possible flakyness (#21641)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR fix #21499.
The vertical axis scale has an inverted range
[max,min]and we need to compute the absolute range.Adds also functional tests for Line, Area and Bar charts that checks the chart visualize correctly all the required Y Axis ticks independently on the scale type.