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

[DatePicker] ❤️ #2789

Merged
merged 11 commits into from
Aug 10, 2018
Merged

[DatePicker] ❤️ #2789

merged 11 commits into from
Aug 10, 2018

Conversation

giladgray
Copy link
Contributor

@giladgray giladgray commented Aug 9, 2018

Changes proposed in this pull request:

  • just a big tender massage to basically all of the DatePicker internals without affecting the appearance or usability
  • major focus on using const over let
  • the biggest code change is the introduction of updateValue() to manipulate this.state.value and standardize the handler impls

Reviewers should focus on:

  • see the individual commits, I've been careful

@blueprint-bot
Copy link

areSameDay calls areSameMonth

Preview: documentation | landing | table

Copy link
Contributor

@cmslewis cmslewis left a comment

Choose a reason for hiding this comment

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

Approved with minor nits

selectedDay = nextProps.value.getDate();
super.componentWillReceiveProps(nextProps);
const { value } = nextProps;
if (value !== this.props.value) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: to avoid deep nesting in these situations, I prefer to handle easy/erroneous cases first to achieve a flatter, easier-to-read code structure:

if (value === this.props.value) {
    // no action needed
    return;
}

if (value == null) {
    // clear the value
    this.setState({ value });
}

// otherwise, use the new value
this.setState({
    displayMonth: value.getMonth(),
    displayYear: value.getFullYear(),
    selectedDay: value.getDate(),
    value,
});

const displayMonth = day.getMonth();
const displayYear = day.getFullYear();
const selectedDay = day.getDate();
if (!modifiers.disabled) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Same

}
};

private computeValidDateInSpecifiedMonthYear(displayYear: number, displayMonth: number): Date {
const { minDate, maxDate } = this.props;
// month is 0-based, date is 1-based. date 0 is last day of previous month.
Copy link
Contributor

Choose a reason for hiding this comment

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

RE: date 0 is last day of previous month: how so? is that actually how the API works?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure seems to be!

packages/datetime/src/datePicker.tsx Show resolved Hide resolved
};

private handleYearSelectChange = (displayYear: number) => {
let { displayMonth, value } = this.state;
let displayMonth = this.state.displayMonth;
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: destructure: let { displayMonth } = this.state;

@blueprint-bot
Copy link

fix tests

Preview: documentation | landing | table

@giladgray giladgray merged commit 1280b13 into develop Aug 10, 2018
@giladgray giladgray deleted the gg/datepicker-love branch August 10, 2018 19:45
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants