Feature/datepicker by snide · Pull Request #644 · elastic/eui

snide · 2018-04-10T05:07:58Z

Summary

Adds a Datepicker to EUI based on react-datepicker. The SCSS layer has been heavily modified while still keeping the original selectors (with some minor wrapper additions when needed). It works within our form system / validation controls. It removes some unneeded features from react-datepicker by spitting out warnings if certain props are used.

Status

This is mostly working, but needs some documentation and code cleanup.

Date works, time works, date time works
Two input date ranges work
Inline abilities should work for the way we need them in Caroline's mocks
Dark mode works
The features listed from the doc examples in the todo below should all be working / styled correctly, they just need docs.
The features we don't plan on using from react-datepicker (like withPortal) should properly spit out errors and be excluded from use

TODO

snide · 2018-04-11T22:36:27Z

@nreese @cchaos OK. I think this thing is mostly done and ready for review. I'm gonna push on doing the range wrapper stuff in a different PR since it'll likely require another component, but the ranges as is work good as separated items.

@cchaos feel free to make any visual changes directly to the PR if you want to clean up any styling

The props documentation is busted, need to dig in an figure out what's going on there.

nreese · 2018-04-12T00:36:53Z

I am unable to cleaning run yarn run build on this branch. Without being able to run a build, I can not try out the datepicker in kibana.

snide · 2018-04-12T04:31:05Z

@nreese Fixed. Took me longer than I'd care to admit to find the problem. Basically I was trying to replicate a css selector bug in react-datepicker, but the sass compiler (weirdly only in the build step) didn't like it. I made the selector a different way (that is still pretty silly) but works.

I need to submit a fix to them upstream.

nreese · 2018-04-12T18:51:56Z

is it possible to have the inline date picker still show the input?

cchaos · 2018-04-12T19:57:16Z

Maybe on your monitor it's hard to see, but I've blown it up a bit. Take a look at the right arrow. This is the hover state:

snide · 2018-04-12T20:42:53Z

is it possible to have the inline date picker still show the input?

I think you'd achieve that by passing the datepicker state to a separate EuiText component. There's no way to natively do that with this library.

snide · 2018-04-12T21:55:40Z

OK. Styley stuff @cchaos mentioned should all be fixed. Gonna leave the classname stuff for now.

nreese

looking great

nreese · 2018-04-12T22:10:14Z

package.json

    "prop-types": "^15.6.0",
    "react-ace": "^5.5.0",
    "react-color": "^2.13.8",
+    "react-datepicker": "v1.4.0",


1.4.1 was released a few days ago. Might as well start out up date.

nreese · 2018-04-12T22:13:10Z

src/components/date_picker/date_picker.js

+} from '../error_boundary';
+
+export class EuiDatePicker extends Component {
+  constructor(props) {


constructor can be removed. state.startDate is never used and handleChange is also never used

nreese · 2018-04-12T22:13:39Z

src/components/date_picker/date_picker.js

+    this.handleChange = this.handleChange.bind(this);
+  }
+
+  handleChange(date) {


handleChange can be removed. It is never used

nreese · 2018-04-12T22:16:25Z

src/components/date_picker/date_picker.js

+      // There is no reason to launch the datepicker in its own modal. Can always build these ourselves
+      this.props.withPortal
+    ) {
+      datePickerOrError = (


Instead of overriding datePickerOrError, why not just return error components? Maybe move if statement ahead of date picker definition so code reads like this

if (using unused props) { return (<div>RTFM</div>) } return ( <span className={classes}> <EuiFormControlLayout> <EuiValidatableControl> <DatePicker/> </EuiValidatableControl> </EuiFormControlLayout> </span> )

nreese · 2018-04-12T22:32:16Z

@snide I figured out why props is failing in docs. shouldCloseOnSelect is given a default value but is not defined as a prop. Looks like it needs to be added EuiDatePicker.propTypes definition. Same with shadow

snide · 2018-04-12T22:49:23Z

@nreese yessss.... feels good man

nreese

lgtm

cchaos

Whew! I don't see anything in the code that really sticks out. I would consider commenting about the commented out code either as to why we're not using it and/or why we're keeping it in but commented.

cchaos · 2018-04-12T22:59:40Z

src-docs/src/views/date_picker/date_picker_example.js

+        The passed icon needs to come from our list of svg icons. Can be flipped {
+          // eslint-disable-next-line react/no-unescaped-entities
+        } to the other side by passing <EuiCode>iconSide="right"</EuiCode>.
+      </p>


Looks like duplicate description text from the Icon section

cchaos · 2018-04-12T23:02:30Z

src/components/date_picker/_date_picker.scss

+
+
+// We use SVG URIs for the navigation arrows. There is one for light and dark.
+@mixin datePicker__arrow {


Is this specific to the datepicker or should we make it a general caret + direction mixin?

It's specific. Normally we have control over the dom and would use an EuiButtonIcon. I'm injecting this into their dom. I don't think we'd ever want to do something like this. I'll make the comment more clear.

Ok, I know that we have some svg uri stuff in /global_styling/mixins/_icons.scss so I wasn't sure if we wanted to plop this in there too.

cchaos · 2018-04-12T23:03:28Z

src/components/date_picker/_date_picker.scss

+    .react-datepicker-popper {
+      @include euiBottomShadowMedium;
+
+      border: 1px solid $euiBorderColor;


Use $euiBorderThin here?

And there's other instances throughout that could be changed as well.

cchaos · 2018-04-12T23:04:10Z

src/components/date_picker/_date_picker.scss

+        background: $euiColorLightestShade;
+    }
+
+    // If the shadow is on, and it is inline, we need to put the shadow on the datepciker


datepicker typo

cchaos · 2018-04-12T23:05:43Z

src/components/date_picker/_date_picker.scss

+    box-shadow:
+      0 0px 12px -1px rgba($euiShadowColor, .2),
+      0 0px 4px -1px rgba($euiShadowColor, .2),
+      0 0px 2px 0 rgba($euiShadowColor, .2) !important;


Can you make a shadow mixin for this, I can see reuse.

cchaos · 2018-04-12T23:12:29Z

Ha... sorry, the left arrow still has the circle in a border issue.

cchaos · 2018-04-12T23:14:28Z

And I still see the custom class stuff in the docs. But other than that. I think it's good to merge!

Congrats!

snide · 2018-04-12T23:36:10Z

Thanks for the review y'all.

snide added feature request labels Apr 10, 2018

snide requested a review from nreese April 11, 2018 20:50

snide force-pushed the feature/datepicker branch from fd68b6c to ecbf86b Compare April 11, 2018 20:53

snide requested a review from cchaos April 11, 2018 20:54

snide removed the work in progress label Apr 11, 2018

snide changed the title ~~Feature/datepicker - WIP~~ Feature/datepicker Apr 11, 2018

snide mentioned this pull request Apr 12, 2018

Datetime selector prototype #226

Closed

snide added 19 commits April 12, 2018 10:49

start of datepicker

0049577

update datepicker, now works with form controls

85602b3

more styling

f2e0602

better timebox

ee79117

shadows are optional, inline works

f17a930

ranges work

307f523

blacklist certain react-datepicker props, set defaults for others

fbd5b8c

more props alignment

e4b27d0

cleanup css / prop layer

d2fc347

docs for states and locale

8391215

time selection docs

231dfc7

validation examples

7b2ce38

min and max times

59b67ff

excluding date / time docs

a44b912

cleaning up examples

5858ca6

more documentation

cd6ea5b

custom inputs

5ce2f64

some responsive adjustments for mobile

84086fc

cleanup sass, fix errors

96516eb

snide added 6 commits April 12, 2018 13:58

address design feedback

1740893

fix goofy border with a box shadow

f0eb713

use clock icon if only time select, format date properly

19e67ba

clean up some more styling, address feedback

968e1b6

make time picker dropdown look similar to combo box

96fd063

more time only styling

aef7b8d

nreese reviewed Apr 12, 2018

View reviewed changes

snide added 3 commits April 12, 2018 15:25

update to react-datepicker@1.4.1

1e53815

remove constructor, address feedback

cab7fb4

clean up the way errors are returned

1ec01ac

fix props doc error, add props docs

3ffc72b

nreese approved these changes Apr 12, 2018

View reviewed changes

cchaos reviewed Apr 12, 2018

View reviewed changes

changelog

d68e2f6

cchaos approved these changes Apr 12, 2018

View reviewed changes

snide added 3 commits April 12, 2018 16:25

address feedback

43826fc

pesky outline

ca03480

docs and borders

9d22f33

snide merged commit 143f89c into elastic:master Apr 12, 2018

snide deleted the feature/datepicker branch April 12, 2018 23:36

snide mentioned this pull request Apr 12, 2018

Time picker into react elastic/kibana#15212

Closed



		// We use SVG URIs for the navigation arrows. There is one for light and dark.
		@mixin datePicker__arrow {

Comments

Conversation

snide commented Apr 10, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Status

TODO

Uh oh!

snide commented Apr 11, 2018

Uh oh!

nreese commented Apr 12, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

snide commented Apr 12, 2018

Uh oh!

nreese commented Apr 12, 2018

Uh oh!

cchaos commented Apr 12, 2018

Uh oh!

snide commented Apr 12, 2018

Uh oh!

snide commented Apr 12, 2018

Uh oh!

nreese left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nreese commented Apr 12, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

snide commented Apr 12, 2018

Uh oh!

nreese left a comment

Choose a reason for hiding this comment

Uh oh!

cchaos left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cchaos commented Apr 12, 2018

Uh oh!

cchaos commented Apr 12, 2018

Uh oh!

snide commented Apr 12, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

snide commented Apr 10, 2018 •

edited

Loading

nreese commented Apr 12, 2018 •

edited

Loading

nreese commented Apr 12, 2018 •

edited

Loading