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.

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

[DateRangePicker] support time selection #2895

Merged

Conversation

Sam-Kramer
Copy link
Contributor

Fixes #523

Checklist

Changes proposed in this pull request:

A TimePicker is added so a time can be specified for a given time range

Reviewers should focus on:

My updates to the DateRangePicker tests and handleTimeChange

@palantirtech
Copy link
Member

Thanks for your interest in palantir/blueprint, @Sam-Kramer! Before we can accept your pull request, you need to sign our contributor license agreement - just visit https://cla.palantir.com/ and follow the instructions. Once you sign, I'll automatically update this pull request.

@giladgray giladgray self-requested a review August 31, 2018 17:56
Copy link
Contributor

@giladgray giladgray left a comment

Choose a reason for hiding this comment

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

please enable circle and join the gh org

@@ -31,6 +31,7 @@ export const DATERANGEPICKER_SINGLE_MONTH = `${DATERANGEPICKER}-single-month`;
export const DATERANGEPICKER_DAY_SELECTED_RANGE = `${DATEPICKER_DAY}--selected-range`;
export const DATERANGEPICKER_DAY_HOVERED_RANGE = `${DATEPICKER_DAY}--hovered-range`;
export const DATERANGEPICKER_SHORTCUTS = `${DATERANGEPICKER}-shortcuts`;
export const DATERANGEPICKER_TIMEPICKER = "DateRange-TimePickers";
Copy link
Contributor

Choose a reason for hiding this comment

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

this is not a valid class--does not follow the format of those above, nor does it match our classname pattern, nor does the const match the value (singular -> plural)

@@ -105,6 +105,8 @@ export interface IDateRangePickerProps extends IDatePickerBaseProps, IProps {
* If this prop is provided, the component acts in a controlled manner.
*/
value?: DateRange;

timePickerProps?: ITimePickerProps;
Copy link
Contributor

@giladgray giladgray Aug 31, 2018

Choose a reason for hiding this comment

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

this is not needed. it's already defined in IDatePickerBaseProps

@@ -203,7 +208,10 @@ export class DateRangePicker extends AbstractPureComponent<IDateRangePickerProps
return (
<div className={classes}>
{this.maybeRenderShortcuts()}
{this.renderCalendars(isShowingOneMonth)}
<div className={classNames(DateClasses.DATEPICKER, className)}>
Copy link
Contributor

Choose a reason for hiding this comment

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

wrong place for classname - it's already set on line 202

Utils.safeInvoke(this.props.timePickerProps.onChange, newTime);
const { value, time } = this.state;
const newValue = DateUtils.getDateTime(
value[dateIndex] != null ? new Date(value[dateIndex].getTime()) : new Date(),
Copy link
Contributor

Choose a reason for hiding this comment

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

use DateUtils.clone()

<Icon icon="arrow-right" />
<MomentDate date={end} format={format} />
<MomentDate withTime={true} date={end} format={format} />
Copy link
Contributor

Choose a reason for hiding this comment

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

ummm pass the withTime prop here please

return (
<div className={DateClasses.DATERANGEPICKER_TIMEPICKER}>
<TimePicker
key="left"
Copy link
Contributor

Choose a reason for hiding this comment

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

keys are not needed here -- only in an array

@giladgray giladgray changed the title Sk/date range picker time select [DateRangePicker] support time selection Aug 31, 2018
@Sam-Kramer
Copy link
Contributor Author

renamed timepicker classes appropiately, and code cleanup

Preview: documentation | landing | table

@giladgray
Copy link
Contributor

giladgray commented Sep 5, 2018

@Sam-Kramer feels good when playing with the preview -- preserves date and time as expected. please just fix those withTime={true}s

@Sam-Kramer
Copy link
Contributor Author

updated momentTime to pass props

Preview: documentation | landing | table

@giladgray
Copy link
Contributor

@Sam-Kramer there's one minor issue: clicking on a shortcut in the left column uses the current time and ignores the two inputs. i think we should either a) change the inputs to show the current time or b) change the time to respect the inputs. thoughts?

@Sam-Kramer
Copy link
Contributor Author

@giladgray Good catch! I like option b the most, as it preserves the user's input. May cause confusion to end users if there time is randomly changed / updates.

@giladgray
Copy link
Contributor

@Sam-Kramer think you can implement that feature here and add a test or two?

@Sam-Kramer
Copy link
Contributor Author

Yup!

@Sam-Kramer
Copy link
Contributor Author

clicking on a shortcut doesn't change the time

Preview: documentation | landing | table

@Sam-Kramer
Copy link
Contributor Author

removed only from describe in tests

Preview: documentation | landing | table

.#{$ns}-daterangepicker-timepickers {
display: flex;
justify-content: space-around;
clear: both;
Copy link
Contributor

Choose a reason for hiding this comment

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

@Sam-Kramer why did you add this property? it has no effect since we don't use floats.

@@ -113,6 +113,7 @@ export interface IDateRangePickerState {
leftView?: MonthAndYear;
rightView?: MonthAndYear;
value?: DateRange;
time?: DateRange;
Copy link
Contributor

Choose a reason for hiding this comment

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

my final question is... can this feature be implemented without adding this time state? i was able to do so in DatePicker by simply treating the regular value as both date and time (cuz it is).

…e is preserved when un-selecting / reselecting date
@Sam-Kramer
Copy link
Contributor Author

removed unneeded CSS property and added test for making sure that time is preserved when un-selecting / reselecting date

Preview: documentation | landing | table

@giladgray giladgray merged commit 293d5a0 into palantir:develop Sep 7, 2018
NachoJusticia added a commit to graphext/blueprint that referenced this pull request Sep 10, 2018
* Fix tabs querySelector on jsdom (palantir#2761)

* match event handler prop name with code and docs (palantir#2760)

* matched icons with the original design (palantir#2780)

* [Popover] captureDismiss=false by default  (palantir#2776)

* disable captureDismiss by default as it breaks links

* fix tests

* redirect old v1, v2 sites to new URLs (palantir#2773)

* Remove `HACKHACK` (palantir#2790)

BP requires `"@types/react": "^16.0.34"` and it is pinned by `yarn.lock`.

For this version of typings, it is provided an overload, then the `HACKHACK` does not persist.

```ts
    function createElement<P>(
        type: SFC<P> | ComponentClass<P> | string,
        props?: Attributes & P,
        ...children: ReactNode[]): ReactElement<P>;
```

See issue: palantir#2785

* [RadioGroup] [HtmlSelect] options support className and disabled (palantir#2783)

* Fix: <RadioGroup> passes custom className to options (+ test)

* Fix: <HtmlSelect> passes custom className to options (+ test)

* HTMLSelect supports disabled

* test all options props

* remove className test

* [DatePicker] ❤️  (palantir#2789)

* tests for DateUtils.getDateTime()

* replace setStateWithValueIfUncontrolled with setState & updateValue helper

refactor handlers to reduce let vars and clarify logic

* refactor constructor into helper functions

* merge disabled checks

* fix test for initial state

* massage imports

* little bug fixes

- ignoring next month change when null
- correct day calculation
- comments!

* bump react-day-picker

* areSameDay calls areSameMonth

* reduce nesting

* fix tests

* Add Popover support for "auto-start" and "auto-end" (palantir#2772)

* palantir#2770: Popover now accepts 'auto-start' and 'auto-end' positions

* Update example

* Update docs

* Oops, undo unintended changes to example

* [Button] Utils.isEmptyReactNode solves icon regression (palantir#2775)

* Utils.isEmptyReactNode solves button regression

* naming and test

* rename in tests

* Publish

 - @blueprintjs/core@3.3.0

* updated Sketch file (palantir#2813)

* [TagInput] On paste, don't tag-ify text if no separator included (palantir#2804)

* [TagInput] Leave a delimiter-less value in the input on paste

* Update tests

* Update docs

* [DateRangePicker] all tests use enzyme (palantir#2793)

* DRP tests use enzyme everywhere with a cool harness

* name clash

* fix tests in React 15 by using accessor to find latest element when needed

* [new] Divider component (palantir#2854)

* add Divider component

* example

* docs edits

* replace all modifiers with borders

* ignore coverage

* remove fill from example

* english is hard

* [DatePicker] reuse existing components in caption (palantir#2792)

* HTMLSelect: add iconProps, fix dark icon color

* use HTMLSelect in caption

massive reduction in styles

* DatePickerNavbar renders prev/next Buttons

use Button for another reduction in styles

* cache month widths

* DatePicker uses single handleMonthChange for all DayPicker events

caption (month/year select) and navbar (prev/next buttons) now all use the same logic for changes!

* add DPNavbar to DRP

* fix & refactor caption & DP tests

assertSelectedDays() helper in DP tests replaces getSelectedDays()

* add $datepicker-padding variable

* adjust paddings to use standard buttons in navbar

* month icon won't exceed select bounds

* dateinput tests

* renames

* replace borders with Divider elements

also remove all negative margins

* copyright, test helper

* use Divider component

* fix DRP tests

* adjust month select icon position

* margin only on caption

* fix R15 tests

* [DatePicker] time support: timePrecision & timePickerProps (palantir#2856)

* add TimePicker props right in DatePicker!

* getDateTime() to merge date and time

* remove "none" from allowed timePrecision

* add style for TP in DP

* refactors to MomentDate and PrecisionSelect to support time

* add PrecisionSelect to DP and DRP examples

* import types, update styles (no divider)

* revert some example changes

* check null in caption

* revert DRP example change

* DateInput only renders DatePicker

* deprecate DateTimePicker

* fix dateinput test

* tests for time!

* strict boolean, no only

* top margin on timepicker

* refactors to reduce some complexity (palantir#2858)

* [DateRangePicker] Shortcuts component and renderCalendars method (palantir#2859)

* Shortcuts component and renderCalendars method

to greatly simplify render()

* bind handleNextState

* Publish

 - @blueprintjs/icons@3.1.0

* remove dependencies section

* [table] fix invisible table menu icon (palantir#2866)

Fixes palantir#2865

* [Spinner] restore IE support (palantir#2868)

* fix Spinner in IE by adding HTML wrapper tag for the animation

* fix loading button spinner position

* add tagName test

* attempt to fix changing value on IE

* added latest version (palantir#2862)

* Publish

 - @blueprintjs/core@3.4.0
 - @blueprintjs/table@3.1.1

* sketch file updated date

* sandbox link in readme

* remove quotes on $ns variable value (palantir#2881)

* [Icon] render HTML element & tagName prop (palantir#2884)

* Icon tagName prop and set `.bp3-icon-{name}` on element

* refactor icon styles so svg is child

- only render font glyph if the icon element is :empty

* icon docs

* color prop becomes fill attribute on svg, overrides css colors

* fix tests

* oops fix icon classes on non-icon elements (like callouts & buttons)

* fix text ref (palantir#2888)

* Skeleton: fix FF support! (palantir#2887)

and refactor styles for simpler keyframes

* [OverflowList] fix browser zoom behavior (palantir#2886)

* fix OverflowList when zoomed

* less magical number

* [docs] better version tag styles (palantir#2889)

* [docs] better version tag styles

* remove obsolete flex override on tag icons

* center docs-nav-buttons and key-combo

*  [Skeleton] Increase animation contrast (palantir#2885)

* [Spinner] add additional child element to isolate spinner from parent (palantir#2890)

* add spinner-animation element to isolate spinner from parent

* comments about elements

* Publish

 - @blueprintjs/core@3.5.0

* [Icon] revert to inline-block (palantir#2896)

* clean up comments

* take no chances

* revert to inline-block

and set block on svg instead of relying on flex child

fixes all noted regressions

* Publish

 - @blueprintjs/core@3.5.1

* Incorrect argument name (palantir#2898)

Copied this over and realized that the argument should not be item but rather film.

* [Suggest] Added selectedItem prop (palantir#2874)

* Added selected item prop on the ISuggestProps def.

* Init the Suggest state with the selected item prop.

* Fixed Suggest support for controlled mode.

* Fixed a tslint coma issue.

* Made the state the only source of truth, added tests.

* Added more tests, improved controlled mode support

Now the Suggest does not update its underlying state in controlled mode, just like the EditableText component.

* Added support for controlled empty selection.

* prop docs

* whitespace

* [TagInput] Use $input-padding-horizontal when empty for consistency with <InputGroup> (palantir#2900)

* Use -padding-horizontal in empty <TagInput>

* Remove -empty class, use pure CSS approach

* Add left-icon toggle to example

* 🔧 switch to tree-sitter-typescript (palantir#2908)

* switch to tree-sitter-typescript, move syntax pkgs to docs-data

* update syntax tokens

* 🔧 switch to circle-github-bot (palantir#2907)

* add circle-github-bot

* new preview script

* restore cache in circle job

* delete old scripts

* [timepicker] Fix allowing to type time that exceeds time bounds (palantir#2795)

* [DateRangePicker] support time selection (palantir#2895)

* add time selection unit tests

* add maybeRender placeholder

* add wrapper div for calendars + time

* add time precision to DRP example

* reorder function

* DateRangePicker now has time selection field

* Updated DateRangePicker example to show time when precision is selected

* Updated props to include timePickerProps and fixed/cleaned up tests for DateRangePicker

* renamed timepicker classes appropiately, and code cleanup

* updated momentTime to pass props

* clicking on a shortcut doesn't change the time

* removed only from describe in tests

* removed unneeded CSS property and added test for making sure that time is preserved when un-selecting / reselecting date

* Um/fix collapse animation (palantir#2911)

* fixes collapse opening animation on first open

* fix documentation

* Add condensed property to HTML tables (palantir#2904)

* Add condensed property to HTML tables

* Move description to small instead of condensed

* Deprecate small property on HTML tables

* comment format

* move collapse animation state docs onto the enum values, CLOSING_END -> CLOSING (palantir#2914)

* bump sass-inline-svg (palantir#2915)

* [docs] Modifiers & update DTP deprecation (palantir#2909)

* add modifiers docs

* update DTP deprecation notice

* back to present tense since we're ready to ship

* [Switch] fix switch styles variables for dark theme (palantir#2912)

* [Select] Add resetOnQuery prop (palantir#2894)

* [Select] Add resetActiveItemOnQuery prop

* Switch prop name from resetActiveItemOnQuery to resetOnQuery and default prop to true

* Match documentation to current functionality

* Move default prop down to the lowest level, queryList
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants