Skip to content
This repository has been archived by the owner on Dec 10, 2021. It is now read-only.

feat(time-format): add full-date to weekly time formatter #486

Merged
merged 3 commits into from
May 14, 2020

Conversation

ktmud
Copy link
Contributor

@ktmud ktmud commented May 13, 2020

🏆 Enhancements

BigNumber trendline uses time formatter based on granularity. Currently the formatter weekly periods is not very helpful. We add full date to the formatted string so users don't have to compute which month/day the week is on their own.

Before

image

After

image

@ktmud ktmud requested a review from a team as a code owner May 13, 2020 18:03
@vercel
Copy link

vercel bot commented May 13, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/superset/superset-ui/2f5dr9a15
✅ Preview: https://superset-ui-git-fork-ktmud-big-number.superset.now.sh

@codecov
Copy link

codecov bot commented May 13, 2020

Codecov Report

Merging #486 into master will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #486      +/-   ##
==========================================
+ Coverage   22.73%   22.75%   +0.01%     
==========================================
  Files         276      276              
  Lines        6672     6672              
  Branches      645      645              
==========================================
+ Hits         1517     1518       +1     
  Misses       5115     5115              
+ Partials       40       39       -1     
Impacted Files Coverage Δ
...t-chart-big-number/src/BigNumber/transformProps.ts 52.00% <ø> (ø)
...at/src/factories/getTimeFormatterForGranularity.ts 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 517f21c...8439215. Read the comment docs.

'1969-12-28T00:00:00Z/P1W': SUNDAY_BASED_WEEK, // 'week_start_sunday'
'1969-12-29T00:00:00Z/P1W': MONDAY_BASED_WEEK, // 'week_start_monday'
'P1W/1970-01-03T00:00:00Z': SUNDAY_BASED_WEEK, // 'week_ending_saturday'
'P1W/1970-01-04T00:00:00Z': MONDAY_BASED_WEEK, // 'week_ending_sunday'
};

export type TimeGranularity = keyof typeof formats;
export type TimeGranularity =
Copy link
Contributor

Choose a reason for hiding this comment

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

There is a known issue with re-exporting type for library.

export {
  default as getTimeFormatterForGranularity,
  TimeGranularity,
} from './factories/getTimeFormatterForGranularity';

there will be issue with TimeGranularity in the consumer app.

Have to create ./types.ts and put this type there
then in index

export * from './types.ts'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does it mean the consumer apps will have to import from '@sueprset-ui/time-format/src/types too?

Copy link
Contributor

Choose a reason for hiding this comment

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

no they can

import { TimeGranularity } from '@superset-ui/time-format';

formData={formData}
formData={{
...formData,
timeGrainSqla: 'P1W', // weekly
Copy link
Contributor

Choose a reason for hiding this comment

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

do you want to make another story for this or remove the change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a new story.

@kristw kristw changed the title feat: add full-date to weekly time formatter feat(time-format): add full-date to weekly time formatter May 14, 2020
@ktmud ktmud merged commit b2d0426 into apache-superset:master May 14, 2020
@ktmud ktmud deleted the big-number branch May 14, 2020 17:44
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants