Added time field to date range widget on 'specific' tab #62
Conversation
There was a problem hiding this comment.
Switching from relative to specific time needs some more work.
I would expect that when you have latest day selected and switch to fixed the same 1 day period is preselected which is not the case now.
In the current version when you switch from latest day to specific at 15:02 you have effectively 25 hours selected: "D-1 15:00" in "Start" and "D 15:59" in "End" boxes.
| this.setState({ | ||
| dateString | ||
| dateString: dateString |
There was a problem hiding this comment.
I would leave it with a shorthand notation as it was before to reduce boilerplate
| renderCustom() { | ||
| var { essence, timekeeper, dimension } = this.props; | ||
| var { startTime, endTime } = this.state; | ||
| renderSpecificDateRangePicker() { |
There was a problem hiding this comment.
Specific seems a bit superfluous to me.
| }); | ||
| } | ||
|
|
||
| dateChange(e: KeyboardEvent) { | ||
| var dateString = (e.target as HTMLInputElement).value.replace(/[^\d-]/g, '').substr(0, 10); | ||
| const dateString = (e.target as HTMLInputElement).value.replace(/[^\d-]/g, '').substr(0, 10); |
There was a problem hiding this comment.
this substring is not needed, user should be able to put longer date than 10 characters temporarily
| } | ||
| } | ||
|
|
||
| changeDate(possibleDateString: string): void { | ||
| timeChange(e: KeyboardEvent) { | ||
| const timeString = (e.target as HTMLInputElement).value.replace(/[^(\d|:)-]/g, '').substr(0, 10); |
There was a problem hiding this comment.
this substring is not needed, user should be able to put longer time than 10 characters temporarily
| timeString: timeString | ||
| }); | ||
|
|
||
| if (timeString.length === 5) { |
There was a problem hiding this comment.
it is not necessarily, final possibleMoment is validated later using isValid method
| this.setState({ | ||
| dateString | ||
| dateString: dateString | ||
| }); | ||
|
|
||
| if (dateString.length === 10) { |
There was a problem hiding this comment.
it is not necessarily, final possibleMoment is validated later using isValid method
|
|
||
| if (!possibleMoment.isValid()) { | ||
| onChange(null); | ||
| } else { | ||
| // add one if end so it passes the inclusive formatting | ||
| if (type === "end") { | ||
| possibleMoment.add(1, "day" ); | ||
| // possibleMoment.add(1, "day" ); |
There was a problem hiding this comment.
end date or end time should be exclusive, for one day period it should be:
from: 10-10-2017 00:00
to: 11-10-2017 00:00
| if (!dimension) return null; | ||
| const menuSize = Stage.fromSize(MENU_WIDTH, 410); | ||
|
|
||
| var tabs = ['relative', 'specific'].map((name) => { | ||
| const tabs = ['relative', 'specific'].map((name) => { |
There was a problem hiding this comment.
please change "specific" into fixed, "fixed" is much better and consistent with "copy url - fixed time" under share menu
src/common/utils/time/time.ts
Outdated
| @@ -167,7 +167,16 @@ export function getWallTimeString(date: Date, timezone: Timezone, includeTime?: | |||
| if (includeTime) { | |||
| return wallTimeISOString.replace('T', delimiter || ', '); | |||
| } | |||
| return wallTimeISOString.replace( /:\d\d/, '').split('T')[0]; | |||
| return splitDateTimeString(wallTimeISOString)[0] | |||
There was a problem hiding this comment.
change date -> format -> custom parse to get date or time
to
date -> format into date
date -> format into time
directly
rafalnowak
force-pushed
the
date_range_with_time
branch
from
5e1ba3e to
a887f45
Compare
src/common/utils/time/time.ts
Outdated
| @@ -222,3 +219,16 @@ export function formatTimeBasedOnGranularity(range: TimeRange, granularity: Dura | |||
| export function formatGranularity(granularity: string): string { | |||
| return granularity.replace(/^PT?/, ''); | |||
| } | |||
|
|
|||
| export function isFullyDefinedDate(date: string): boolean { | |||
There was a problem hiding this comment.
I would probably change is to maybe as the function tests necessity rather than sufficiency
src/common/utils/time/time.ts
Outdated
| return date.length === FORMAT_DATE.length; | ||
| } | ||
|
|
||
| export function isFullyDefinedTime(time: string): boolean { |
There was a problem hiding this comment.
I would probably change is to maybe as the function tests necessity rather than sufficiency
Added time field to date range widget on 'specific' tab.
Fixes #33