Skip to content

Commit

Permalink
Fixed API types inconsistency for time values
Browse files Browse the repository at this point in the history
  • Loading branch information
timocov committed Jun 8, 2020
1 parent 8e788d6 commit 8675422
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 10 deletions.
8 changes: 4 additions & 4 deletions docs/time-scale.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ Time scale (or time axis) is a horizontal scale at the bottom of the chart that
|`visible`|`boolean`|`true`|If true, the time scale is shown on a chart|
|`timeVisible`|`boolean`|`false`|If true, the time is shown on the time scale and in the vertical crosshair label|
|`secondsVisible`|`boolean`|`true`|If true, seconds are shown on the label of the crosshair vertical line in `hh:mm:ss` format on intraday intervals|
|`tickMarkFormatter`|`(TimePoint, TickMarkType, locale) => string`|Default tick marks formatter|Allows to override the tick marks formatter (see below)|
|`tickMarkFormatter`|`(TimePoint, TickMarkType, locale) => string` | `undefined`|`undefined`|Allows to override the tick marks formatter (see below)|

### Tick marks formatter

Expand All @@ -25,12 +25,12 @@ Tick marks formatter can be used to customize tick marks labels on the time axis
To customize it, you need to provide the `tickMarkFormatter` option. It's a function with the following declaration:

```typescript
export type TickMarkFormatter = (timePoint: TimePoint, tickMarkType: TickMarkType, locale: string) => string;
export type TickMarkFormatter = (time: UTCTimestamp | BusinessDay, tickMarkType: TickMarkType, locale: string) => string;
```

Where `timePoint` is [Time](./time.md) object, `type` is [TickMarkType](./constants.md#TickMarkType) enum and `locale` is the currently applied locale of the string type.
Where `time` is [Time](./time.md) object, `type` is [TickMarkType](./constants.md#TickMarkType) enum and `locale` is the currently applied locale of the string type.

This function should return `timePoint` as a string formatted according to `tickMarkType` type (year, month, etc) and `locale`.
This function should return `time` as a string formatted according to `tickMarkType` type (year, month, etc) and `locale`.

Note that the returned string should be the shortest possible value and should have no more than 8 characters.
Otherwise, the tick marks will overlap each other.
Expand Down
2 changes: 0 additions & 2 deletions src/api/options/time-scale-options-defaults.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { defaultTickMarkFormatter } from '../../model/default-tick-mark-formatter';
import { TimeScaleOptions } from '../../model/time-scale';

export const timeScaleOptionsDefaults: TimeScaleOptions = {
Expand All @@ -12,5 +11,4 @@ export const timeScaleOptionsDefaults: TimeScaleOptions = {
visible: true,
timeVisible: false,
secondsVisible: true,
tickMarkFormatter: defaultTickMarkFormatter,
};
20 changes: 16 additions & 4 deletions src/model/time-scale.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,12 @@ import { DeepPartial, isInteger, merge } from '../helpers/strict-type-checks';

import { ChartModel } from './chart-model';
import { Coordinate } from './coordinate';
import { defaultTickMarkFormatter } from './default-tick-mark-formatter';
import { FormattedLabelsCache } from './formatted-labels-cache';
import { LocalizationOptions } from './localization-options';
import { areRangesEqual, RangeImpl } from './range-impl';
import { TickMarks } from './tick-marks';
import { Logical, LogicalRange, SeriesItemsIndexesRange, TickMark, TimedValue, TimePoint, TimePointIndex, TimePointsRange } from './time-data';
import { BusinessDay, Logical, LogicalRange, SeriesItemsIndexesRange, TickMark, TimedValue, TimePoint, TimePointIndex, TimePointsRange, UTCTimestamp } from './time-data';
import { TimePoints } from './time-points';
import { TimeScaleVisibleRange } from './time-scale-visible-range';

Expand Down Expand Up @@ -53,7 +54,7 @@ export const enum TickMarkType {
TimeWithSeconds,
}

export type TickMarkFormatter = (timePoint: TimePoint, tickMarkType: TickMarkType, locale: string) => string;
export type TickMarkFormatter = (time: UTCTimestamp | BusinessDay, tickMarkType: TickMarkType, locale: string) => string;

export interface TimeScaleOptions {
rightOffset: number;
Expand All @@ -66,7 +67,7 @@ export interface TimeScaleOptions {
visible: boolean;
timeVisible: boolean;
secondsVisible: boolean;
tickMarkFormatter: TickMarkFormatter;
tickMarkFormatter?: TickMarkFormatter;
}

export class TimeScale {
Expand Down Expand Up @@ -727,7 +728,18 @@ export class TimeScale {
tickMarkType = TickMarkType.Year;
}

return this._options.tickMarkFormatter(timePoint, tickMarkType, this._localizationOptions.locale);
if (this._options.tickMarkFormatter !== undefined) {
// this is temporary solution to make more consistency API
// it looks like that all time types in API should have the same form
// but for know defaultTickMarkFormatter is on model level and can't determine whether passed time is business day or UTCTimestamp
// because type guards are declared on API level
// in other hand, type guards couldn't be declared on model level so far
// because they are know about string representation of business day ¯\_(ツ)_/¯
// let's fix in for all cases for the whole API
return this._options.tickMarkFormatter(timePoint.businessDay ?? timePoint.timestamp, tickMarkType, this._localizationOptions.locale);
}

return defaultTickMarkFormatter(timePoint, tickMarkType, this._localizationOptions.locale);
}

private _setVisibleRange(newVisibleRange: TimeScaleVisibleRange): void {
Expand Down

0 comments on commit 8675422

Please sign in to comment.