fix(debug): add predictable axis labels sorting order #1418
Conversation
packages/charts/src/chart_types/xy_chart/state/selectors/get_debug_state.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
LGTM! One minor optional code comment. Also, the sorting order is conditionally ascending or descending; it may be simpler for both our side and the recipient side to always do the same sorting, eg. NumAsc, because the NumDesc is arbitrary/artificial anyway. I'm not aware of a reason for complicating the order with "business logic" for such a debug step, and the ascending order is a good standard, as noone cares about the physical screenspace order (imo)
You are right, but I thought it was easier to grasp if we connect that to a "bottom-left" screen space. When we are going to reverse the way we compute the Y position on cartesian we can clean up this code (currently it follows the top-left 0,0 origin, that is actually harder to reason about that a bottom-left one) |
| expect(debugState.axes?.x[0].labels).toEqual(['0', '1', '2', '3', '4', '5', '6', '7', '8', '9', '10']); | ||
| expect(debugState.axes?.y[0].labels).toEqual(['0', '1', '2', '3', '4', '5', '6', '7', '8', '9', '10']); |
There was a problem hiding this comment.
How about toMatchSnapshot?
| expect(debugState.axes?.x[0].labels).toEqual(['0', '1', '2', '3', '4', '5', '6', '7', '8', '9', '10']); | |
| expect(debugState.axes?.y[0].labels).toEqual(['0', '1', '2', '3', '4', '5', '6', '7', '8', '9', '10']); | |
| expect(debugState).toMatchSnapshot(); |
There was a problem hiding this comment.
This would cover other unexpected changes to debug states.
# [37.0.0](v36.0.0...v37.0.0) (2021-10-05) ### Bug Fixes * **debug:** add predictable axis labels sorting order ([#1418](#1418)) ([60fbe7a](60fbe7a)) * **deps:** update dependency @elastic/eui to ^38.1.0 ([#1409](#1409)) ([4ffd018](4ffd018)) * **deps:** update dependency @elastic/eui to ^38.2.0 ([#1416](#1416)) ([34707c3](34707c3)) ### Code Refactoring * cleanup colors ([#1397](#1397)) ([348c061](348c061)) * scale improvements and TS 4.4 ([#1383](#1383)) ([0003bc1](0003bc1)) ### BREAKING CHANGES * `DEFAULT_CHART_MARGINS`, `DEFAULT_GEOMETRY_STYLES`, `DEFAULT_CHART_PADDING` and `DEFAULT_MISSING_COLOR` are no longer exposed as part of the API * The public type varieties for domains are discontinued, in favor of retaining the single `DomainRange` export, which now has a mandatory `{min: number, max: number}`. The developer can supply `NaN` where a finite min, max or both aren't defined (ie. in place of former effective `undefined`). In addition, some console.warn punctuations changed Co-authored-by: Marco Vettorello <vettorello.marco@gmail.com> Co-authored-by: Nick Partridge <nick.ryan.partridge@gmail.com>
|
🎉 This PR is included in version 37.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Summary
This commit adds a predictable label sorting order for axis labels in the debug state.
The configured sorting order should always reflect the order starting from the origin of the axis, independently from the rotation or the axis position.
This is the only order that remains constant and predictable.
Details
These changes are required to fix the functional tests in Kibana, providing a constant order to the axis labels.
Checklist
:xy,:partition):interactions,:axis)