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

feat: nice ticks everywhere #1087

Merged
merged 18 commits into from
Apr 13, 2021
Merged

feat: nice ticks everywhere #1087

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
fe47338
feat: nice ticks everywhere
markov00 Mar 23, 2021
acee82b
feat: add nice prop on scale type
markov00 Mar 29, 2021
521a15f
refactor: compute scale API config separately
markov00 Mar 31, 2021
5aa72fc
test: fix wrong test change
markov00 Apr 1, 2021
6ec6b3e
fix: reduce global scale correctly
markov00 Apr 6, 2021
5d2f1a8
fix: stories
markov00 Apr 6, 2021
c063d22
Merge branch 'master' into 2021_03_23-nice_ticks_everywhere
markov00 Apr 6, 2021
08527a0
chore: fix lint and api docs
markov00 Apr 7, 2021
5120ef8
Merge branch 'master' into 2021_03_23-nice_ticks_everywhere
markov00 Apr 7, 2021
b9db40c
fix: review comments
markov00 Apr 7, 2021
38ca96f
Merge branch '2021_03_23-nice_ticks_everywhere' of github.com:markov0…
markov00 Apr 7, 2021
e0416b5
Merge branch 'master' into 2021_03_23-nice_ticks_everywhere
markov00 Apr 7, 2021
cedf1bb
refactor: rename internal ticks in desiredTickCount
markov00 Apr 8, 2021
c0060d0
refactor: split type and nice into separate props
markov00 Apr 12, 2021
f0a9090
Merge branch '2021_03_23-nice_ticks_everywhere' of github.com:markov0…
markov00 Apr 12, 2021
7062f47
Merge branch 'master' into 2021_03_23-nice_ticks_everywhere
markov00 Apr 12, 2021
2d16951
test: update VRT with negative values
markov00 Apr 12, 2021
df26747
Merge branch 'master' into 2021_03_23-nice_ticks_everywhere
markov00 Apr 13, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
chore: fix lint and api docs
  • Loading branch information
@markov00
markov00 committed Apr 7, 2021
commit 08527a0e9741ec2d665c5cbe039d9e8afe160398
12 changes: 10 additions & 2 deletions api/charts.api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,14 @@ export const AnnotationType: Readonly<{
// @public (undocumented)
export type AnnotationType = $Values<typeof AnnotationType>;

// @public (undocumented)
export interface APIScale<T extends ScaleType> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure of the pros/cons of putting the word API in an element of the API, all things being equal, would prefer a simple, expressive, semantic name. I'd probably not call it Scale because it's currently just a tuple of {type: 'linear' | ..., nice: boolean}

Copy link
Member Author

Choose a reason for hiding this comment

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

Right, any suggestion? what if we don't expose the interface externally on our API, and we reserve the APIScaletype for internal purposes?
The external will look like this:


export interface SeriesScales {
    ...
    xScaleType: XScaleType | { nice: boolean; type: XScaleType };
    yScaleType: ScaleContinuousType | { nice: boolean; type: ScaleContinuousType };
}

Copy link
Contributor

Choose a reason for hiding this comment

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

I think, even for internal use the word "API" in it is a good question, maybe it has a bunch of benefits, eg. knowing which "boundary layer" we talk about, eg. external API, internal API or implementation detail (the latter of which is clear from @internal).

Could namespaces / modules achieve this without resorting to Hungarian notation?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this inception or recursion?

Copy link
Contributor

Choose a reason for hiding this comment

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

image

// (undocumented)
nice: boolean;
// (undocumented)
type: T;
}
monfera marked this conversation as resolved.
Show resolved Hide resolved

// @public (undocumented)
export interface ArcSeriesStyle {
// (undocumented)
Expand Down Expand Up @@ -1670,10 +1678,10 @@ export type SeriesNameFn = (series: XYChartSeriesIdentifier, isTooltip: boolean)
// @public (undocumented)
export interface SeriesScales {
Copy link
Contributor

Choose a reason for hiding this comment

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

Unrelated to the PR:

  • instead of SeriesScales, we might eventually consider a more specific term, eg. ProjectionPair or CartesianPlaneProjections (or a shorter version), to express that it's not some set or array of scales, but it defines the Cartesian plane, with the orthogonal projection pair
  • also, "projection" might work better here, and also in a lot of places where we use scale at the moment; Projection captures the linearity (or how it's non-linear) of a dimension, eg. current ScaleType; while Scale adds the multiplier and usually offset, for example, by using the data domain and the screen pixel range

timeZone?: string;
xScaleType: XScaleType;
xScaleType: XScaleType | APIScale<XScaleType>;
// @deprecated
yScaleToDataExtent?: boolean;
yScaleType: ScaleContinuousType;
yScaleType: ScaleContinuousType | APIScale<ScaleContinuousType>;
}

// @public (undocumented)
Expand Down
2 changes: 2 additions & 0 deletions src/chart_types/specs.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ export {
RectAnnotation,
} from './xy_chart/specs';

export { APIScale } from './xy_chart/scales/types';

export * from './xy_chart/utils/specs';

export { Partition } from './partition_chart/specs';
Expand Down
1 change: 1 addition & 0 deletions src/chart_types/xy_chart/scales/get_api_scales.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/

import { ScaleContinuousType } from '../../../scales';
import { ScaleType } from '../../../scales/constants';
import { BasicSeriesSpec, XScaleType } from '../utils/specs';
Expand Down
1 change: 1 addition & 0 deletions src/chart_types/xy_chart/scales/scale_defaults.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/

import { ScaleType } from '../../../scales/constants';

/** @internal */
Expand Down
1 change: 1 addition & 0 deletions src/chart_types/xy_chart/scales/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/

import { ScaleType } from '../../../scales/constants';

/** @public */
Expand Down
1 change: 1 addition & 0 deletions src/chart_types/xy_chart/state/selectors/compute_axes_geometries.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/

import createCachedSelector from 're-reselect';

import { getChartIdSelector } from '../../../../state/selectors/get_chart_id';
Expand Down
1 change: 1 addition & 0 deletions src/chart_types/xy_chart/state/selectors/get_api_scale_configs.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/

import createCachedSelector from 're-reselect';

import { ScaleContinuousType } from '../../../../scales';
Expand Down
18 changes: 0 additions & 18 deletions src/chart_types/xy_chart/state/selectors/get_data_domain.ts
View file
Open in desktop

This file was deleted.

3 changes: 3 additions & 0 deletions src/mocks/xy/domains.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/

import { XDomain, YDomain } from '../../chart_types/xy_chart/domains/types';
import { getXAPIScale, getYAPIScale } from '../../chart_types/xy_chart/scales/get_api_scales';
import { X_SCALE_DEFAULT, Y_SCALE_DEFAULT } from '../../chart_types/xy_chart/scales/scale_defaults';
Expand All @@ -24,6 +25,7 @@ import { ScaleContinuousType } from '../../scales';
import { ScaleType } from '../../scales/constants';
import { mergePartial, RecursivePartial } from '../../utils/common';

/** @internal */
export class MockXDomain {
private static readonly base: XDomain = {
...X_SCALE_DEFAULT,
Expand All @@ -46,6 +48,7 @@ export class MockXDomain {
}
}

/** @internal */
export class MockYDomain {
private static readonly base: YDomain = {
...Y_SCALE_DEFAULT,
Expand Down