WIP feat(DatePicker): Add DatePicker component

[sjd78]

What: Add the DatePicker component from PF to PF-react.

The PatternFly design is at: http://www.patternfly.org/pattern-library/forms-and-controls/datepicker/

The PatternFly widget is defined at: http://www.patternfly.org/pattern-library/widgets/#bootstrap-datepicker

See issue #232.

Link to Storybook: https://sjd78.github.io/patternfly-react/?selectedKind=DatePicker&selectedStory=DatePicker

Additional issues:

[sjd78]

Part of contributing this component is to take a deeper look at how 3rd party components can be blended in to patternfly-react repo. This DatePicker should cover the existing PatternFly designs, but may still require tweaking to get interaction parity with the base PatternFly widget.

The 3rd party component being consumed is DateTimePicker in the react-widgets package.

At this point, the PR is a work in progress. There are a few things to consider:

• I'm not quite sure how to move forward with the SCSS imports against the 3rd party component. A new scss partial has been added to pull the react-widgets scss in. I got it working by changing the way node-sass handles imports through a custom importer function. It handles mappings in a move obvious way from includes in the package.json file. See sass-loader.js. package.json now references this loader function instead of listing each individual module/library source location.

• I found it necessary to tweak the storybook webpack config to handle imports with sass-loader.js along with the include definitions. To get the full patternfly-react scss imports, that import was also adjusted.

• Tests haven't been added yet.

• Using recompose is awesome, but it leaves the storybook "Show Info" a bit awkward. Any ideas on how to make that look better would be appreciated.

• i18n and l10n seems not to addressed in PatternFly so far, or at least I didn't see it yet. This component deals with dates which is highly dependent on user locale. For now the component just initializes the localization handling to use moment. The storybook fixes moment to use English (en). A knob can be added to the storybook to let people play with how it looks in different locales or with different date formatting.

[sjd78]

@jelkosz, @gregsheremeta, @vojtechszocs - FYI

[coveralls]

Pull Request Test Coverage Report for Build 729

• 0 of 0 (NaN%) changed or added relevant lines in 0 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage remained the same at 65.647%

---

Totals
Change from base Build 708:	0.0%
Covered Lines:	936
Relevant Lines:	1265

---

💛 - Coveralls

[priley86]

@sjd78 nice job!

I haven't had much time to review the others, but re: i18n:
We had started an issue here previously (and referenced some efforts in oVirt 😉):
#72

We would like to avoid introducing i18n/react-intl into this library (and have it handled by the products). This means that all components here should accept any text attributes as messages props (see pagination example). You are 100% correct that we'll need to be extra careful with this on datepicker/timepicker. I would expect that these components should accept timezone and language attributes as props (and downstream can handle setting these accordingly). Here is an example of that in Foreman (full PR).

[sjd78]

@priley86

We would like to avoid introducing i18n/react-intl into this library (and have it handled by the products). This means that all components here should accept any text attributes as messages props

Absolutely! I just patched ovirt-engine-dashboard to remove react-intl all together in favor of product local handling. Keeping patternfly-react as free from prescribed i18n handling is the right way to go.

You are 100% correct that we'll need to be extra careful with this on datepicker/timepicker. I would expect that these components should accept timezone and language attributes as props (and downstream can handle setting these accordingly).

A DatePicker and a TimePicker will always be tied to i18n. There is no way around that. The component does have properties for locale. I should add a knob in the storybook that lets you change it and see differences.

The react-widgets package localization is setup to have the i18n provider be pluggable. They provide impls for globalize and moment. I think the relevant question for patternfly-react is how to pass this through to component consumers.

[serenamarie125]

@sjd78 this is looking great! I noticed that some of the visuals don't match what's currently in core PF https://rawgit.com/patternfly/patternfly/master-dist/dist/tests/bootstrap-datepicker.html, as far as highlight & selection colors, etc

[sjd78]

@serenamarie125 Excellent!

I didn't tweak the visuals much from the underlying component, so it is a bit different then the bootstrap-datepicker spec. I'm expecting a similar way forward as the Time Picker issue (#180).

Once there is some agreement around handling CSS tweaks from the underlying component, I can make the necessary adjustments.

[jgiardino]

Hey @sjd78, I actually defer to @jeff-phillips-18 on this one, since he was more involved in getting sass set up. I'm also curious how we would convert the third-party sass to less.

[jeff-phillips-18]

I'm OK with the changes to be able to import the react-widgets SASS. Better to move the includes into the importer as the list could continue to grow.

For the Show Info in storybook, we've included custom text in other places to show a better story book. The Toolbar Storybook is an example.

[sharvit]

Consider using node-sass-package-importer
I used a different feature from node-sass-magic-importer and the experience was great.

[sjd78]

The tilde import part works the same way, but the other features are interesting.

[sharvit]

Creating a special syntax will make it difficult to import this file for consumers.
I have an open pr for foreman that handle it differently:
https://github.com/theforeman/foreman/pull/5212/files#diff-a58d55bdb5770c78ad512f8e91f8d051R109

[jeff-phillips-18]

I don't expect consumers would import this file, rather they would import the patternfly-react partials. But if there is a better way to avoid the special syntax, that would be better.

[sharvit]

oh you right, it's a bit confusing since they use they same name

[sjd78]

Really the reason I put the "@" there was to indicate that the import is coming from somewhere non-standard. In this case, the bootstrap and fontawesome imports all come from the patternfly package. That was achieved by including those source sass directories directly on the command line.

I though about changing the import to something that webpack would understand natively like:

@import "~patternfly/node_modules/bootstrap-sass/assets/stylesheets/variables" ;

but that seemed worse.

[serenamarie125]

@sjd78 let me know if the CSS tweaks were handled, and I can come back and take a look when it's ready

[jeff-phillips-18]

@sjd78 Do you want to proceed with this PR or shall we close it out?

[sjd78]

Hi @jeff-phillips-18, I won't have time to work on it anytime soon. I'll close it out. Sorry about that.