Skip to content

Commit

Permalink
Fix types in MetricsTimeSeriesChart (#4093)
Browse files Browse the repository at this point in the history
* Fix types in MetricsTimeSeriesChart

* return stringified value in mouseoverTimeFormat
  • Loading branch information
bcolloran authored Feb 21, 2024
1 parent ea2d490 commit 69033df
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 22 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/web-test-code-quality.yml
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ jobs:
if: steps.filter.outputs.common == 'true'
run: |-
npx eslint web-common --quiet
npx svelte-check --workspace web-common --no-tsconfig --ignore "src/features/dashboards/time-series/(MetricsTimeSeriesCharts.svelte|MeasureChart.svelte),src/features/dashboards/time-controls/TimeControls.svelte,src/components/data-graphic/elements/GraphicContext.svelte,src/components/data-graphic/guides/(Axis.svelte|DynamicallyPlacedLabel.svelte|Grid.svelte),src/components/data-graphic/compositions/timestamp-profile/TimestampDetail.svelte,src/components/data-graphic/marks/(ChunkedLine.svelte|HistogramPrimitive.svelte|Line.svelte|MultiMetricMouseoverLabel.svelte),src/components/column-profile/column-types/details/SummaryNumberPlot.svelte"
npx svelte-check --workspace web-common --no-tsconfig --ignore "src/features/dashboards/time-controls/TimeControls.svelte,src/components/data-graphic/elements/GraphicContext.svelte"
- name: lint and type checks for web local
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,8 +153,12 @@
$metricsView?.data?.defaultTimeRange,
);
startValue = adjustedChartValue?.start;
endValue = adjustedChartValue?.end;
if (adjustedChartValue?.start) {
startValue = adjustedChartValue?.start;
}
if (adjustedChartValue?.end) {
endValue = adjustedChartValue?.end;
}
}
$: if (
Expand Down Expand Up @@ -242,9 +246,10 @@
{#each renderedMeasures as measure (measure.name)}
<!-- FIXME: I can't select the big number by the measure id. -->
<!-- for bigNum, catch nulls and convert to undefined. -->

{@const bigNum = totals?.[measure.name] ?? undefined}
{@const comparisonValue = totalsComparisons?.[measure.name]}
{@const bigNum = measure.name ? totals?.[measure.name] : undefined}
{@const comparisonValue = measure.name
? totalsComparisons?.[measure.name]
: undefined}
{@const isValidPercTotal = measure.name
? $isMeasureValidPercentOfTotal(measure.name)
: false}
Expand Down Expand Up @@ -293,10 +298,13 @@
: null}
mouseoverTimeFormat={(value) => {
/** format the date according to the time grain */
return new Date(value).toLocaleDateString(
undefined,
TIME_GRAIN[interval].formatDate,
);

return interval
? new Date(value).toLocaleDateString(
undefined,
TIME_GRAIN[interval].formatDate,
)
: value.toString();
}}
/>
{:else}
Expand Down
21 changes: 13 additions & 8 deletions web-common/src/lib/time/ranges/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -369,11 +369,11 @@ export function getAdjustedChartTime(
start: Date | undefined,
end: Date | undefined,
zone: string,
interval: V1TimeGrain,
timePreset: TimeRangePreset,
defaultTimeRange: string,
interval: V1TimeGrain | undefined,
timePreset: TimeRangePreset | TimeComparisonOption | undefined,
defaultTimeRange: string | undefined,
) {
if (!start || !end)
if (!start || !end || !interval)
return {
start,
end,
Expand All @@ -382,20 +382,25 @@ export function getAdjustedChartTime(
const grainDuration = TIME_GRAIN[interval].duration;
const offsetDuration = getDurationMultiple(grainDuration, 0.45);

let adjustedStart = new Date(start);
let adjustedEnd = new Date(end);

if (timePreset === TimeRangePreset.ALL_TIME) {
// No offset has been applied to All time range so far
// Adjust end according to the interval
start = getStartOfPeriod(start, grainDuration, zone);
start = getOffset(start, offsetDuration, TimeOffsetType.ADD);
adjustedStart = getStartOfPeriod(adjustedStart, grainDuration, zone);
adjustedStart = getOffset(
adjustedStart,
offsetDuration,
TimeOffsetType.ADD,
);
adjustedEnd = getEndOfPeriod(adjustedEnd, grainDuration, zone);
} else if (timePreset && timePreset === TimeRangePreset.DEFAULT) {
// For default presets the iso range can be mixed. There the offset added will be the smallest unit in the range.
// But for the graph we need the offset based on selected grain.
const smallestTimeGrain = getSmallestTimeGrain(defaultTimeRange);
// Only add this if the selected grain is greater than the smallest unit in the iso range
if (isGrainBigger(interval, smallestTimeGrain)) {
if (smallestTimeGrain && isGrainBigger(interval, smallestTimeGrain)) {
adjustedEnd = getEndOfPeriod(adjustedEnd, grainDuration, zone);
}
} else {
Expand All @@ -415,7 +420,7 @@ export function getAdjustedChartTime(
* To get the correct values, we need to remove the local time zone offset
* and add the offset of the selected time zone.
*/
start: addZoneOffset(removeLocalTimezoneOffset(start), zone),
start: addZoneOffset(removeLocalTimezoneOffset(adjustedStart), zone),
end: addZoneOffset(removeLocalTimezoneOffset(adjustedEnd), zone),
};
}
6 changes: 5 additions & 1 deletion web-common/src/lib/time/ranges/iso-ranges.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,11 @@ export function humaniseISODuration(isoDuration: string): string {
return humanISO;
}

export function getSmallestTimeGrain(isoDuration: string) {
export function getSmallestTimeGrain(isoDuration: string | undefined) {
if (isoDuration === undefined) {
return undefined;
}

const duration = Duration.fromISO(isoDuration);
for (const { grain, unit } of PeriodAndUnits) {
if (duration[unit]) {
Expand Down
6 changes: 3 additions & 3 deletions web-common/src/lib/time/transforms/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export function getStartOfPeriod(
referenceTime: Date,
period: Period,
zone = "UTC",
) {
): Date {
const date = DateTime.fromJSDate(referenceTime, { zone });
return date.startOf(TimeUnit[period]).toJSDate();
}
Expand All @@ -38,7 +38,7 @@ export function getEndOfPeriod(
referenceTime: Date,
period: Period,
zone = "UTC",
) {
): Date {
const date = DateTime.fromJSDate(referenceTime, { zone });
return date.endOf(TimeUnit[period]).toJSDate();
}
Expand All @@ -49,7 +49,7 @@ export function getOffset(
duration: string,
direction: TimeOffsetType,
zone = "UTC",
) {
): Date {
const durationObj = Duration.fromISO(duration);
return DateTime.fromJSDate(referenceTime, { zone })
[direction === TimeOffsetType.ADD ? "plus" : "minus"](durationObj)
Expand Down

1 comment on commit 69033df

@github-actions
Copy link
Contributor

Choose a reason for hiding this comment

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

Please sign in to comment.