Adding an input icon to the begining of the DateRangePickerInput #222

AntiFish03 · 2016-12-19T17:39:45Z

I needed a simple icon added to the beginning of the Date Range Picker. And I thought that others might also like a simple decoration / interaction point.

showCalendarIcon defaults to false, added using

<DatePicker
    showCalendarIcon
/>

Interaction uses onStartDateFocus

Calendar Icon was extracted from font-awesome 4.7.0 licensing is fully open source, http://fontawesome.io/license/

Added storybook entry, Readme entry and tests.

coveralls · 2016-12-19T17:52:21Z

Coverage increased (+0.02%) to 86.541% when pulling f5cc0e7 on getaroom:add-calendar-icon into 08906d6 on airbnb:master.

coveralls · 2016-12-19T17:59:01Z

Coverage increased (+0.02%) to 86.541% when pulling f5cc0e7 on getaroom:add-calendar-icon into 08906d6 on airbnb:master.

lencioni · 2016-12-19T18:16:22Z

I imagine that people will want to be able to customize the icon to match their sites. Instead of baking the icon in, it seems better to allow a component to be passed in that would enable people to use whatever decoration they wanted. What do you think @majapw?

AntiFish03 · 2016-12-19T18:19:18Z

I can change that if needed but I just followed the choices made with the arrow and the close button of baking them in.

majapw · 2016-12-20T18:52:30Z

Hi @AntiFish03! I think it makes sense to have it be configurable a la the month navigation buttons but maybe default to the SVG you've provided. Can you also post a screenshot of how this looks?

Thanks!

AntiFish03 · 2016-12-20T18:57:42Z

Here is the screenshot.

AntiFish03 · 2016-12-20T19:08:55Z

Also, I am more than happy to make the requested change give me a few minutes to adjust it.

ljharb · 2016-12-20T23:30:11Z

README.md

 ```
-  showCalendarIcon: PropTypes.bool
+  showInputIcon: PropTypes.bool,
+  customInputIcon: PropTypes.node


could we just have a inputIcon prop that's either null or a node? I'm not sure I see the point in having both.

but maybe default to the SVG you've provided.

If it is both optional and customizable it needs to know when to display, when to display the default, and when to display the custom icon.

ah, i didn't realize there'd be an icon included by default. Might it be simpler to go with a single inputIcon prop, and then export the default icon so that users can pass that in if they want it?

Possibly... Its your call. The icon is an export out of the Font Awesome SVG, under its open license. So its definitely not a custom calendar icon.

However, this is more simple if you are just going to use the basic input icon because trying to use the traditional method of adding to the HTML below and adjusting its positioning could create user interaction difficulties (i.e the icon steals the click).

And this version has a customizable capability that someone with a truly custom calendar icon could use without difficulty.

true - i'm torn. We could do PropTypes.oneOfType([PropTypes.oneOf([true]), PropTypes.node]) but that seems icky.

Wouldn't PropTypes.oneOfType([PropTypes.bool, PropTypes.node]) work. It would be less exact but its also less icky

And thinking about it a little further that would actually be more exact not less, Because false, true, node should all be valid values. We are just choosing to think of it as null, true, node.

Hmm, whatever we choose to use here will likely also end up influencing how we implement customization on the clear dates SVG.

I am happy to make changes in what ever direction... I have a need for this and would rather assist the community using this than maintain a fork...

coveralls · 2016-12-20T23:40:56Z

Coverage increased (+0.04%) to 86.541% when pulling 6363680 on getaroom:add-calendar-icon into fa3da30 on airbnb:master.

AntiFish03 · 2016-12-20T23:52:44Z

I am happy to adjust this however it needs to be to allow the input icon, (so I don't have to maintain a fork).

Whether there is a baked in default or customizable, or both. Just need to know which way to take the code.

majapw · 2017-01-12T21:59:22Z

@AntiFish03
I think the best step would be to leave it as two props but have them be very slightly different. Instead of showInputIcon and customInputIcon, I would have showDefaultInputIcon and customInputIcon. If you have either of the two props, the icon will show. The only difference will be whether it is the default option or the custom option.

For defensive purposes, if both props are there, the custom icon should take precedence.

Does that seem good?

AntiFish03 · 2017-01-21T00:23:34Z

@majapw, I made the changes you suggested.

coveralls · 2017-01-21T00:30:31Z

Coverage increased (+0.04%) to 87.209% when pulling 649a9dd on getaroom:add-calendar-icon into 0efa253 on airbnb:master.

coveralls · 2017-01-21T00:37:18Z

Coverage increased (+0.04%) to 87.209% when pulling 649a9dd on getaroom:add-calendar-icon into 0efa253 on airbnb:master.

coveralls · 2017-01-21T01:19:23Z

Coverage increased (+0.04%) to 87.209% when pulling 3c7b6d1 on getaroom:add-calendar-icon into 0efa253 on airbnb:master.

ljharb · 2017-01-21T06:00:08Z

@AntiFish03 would you mind rebasing on top of latest master on the command line, so that there's no merge commits? If you have trouble, please let us know and we can take care of it for you (but please don't close this PR, delete the branch, or open a new one)

AntiFish03 · 2017-01-23T02:59:12Z

@ljharb, I would but I am away from a consistent internet connection at the moment.

ljharb · 2017-01-23T06:00:45Z

@AntiFish03 np, i've gone ahead and rebased it for you.

@majapw

deferring to @majapw

coveralls · 2017-01-23T06:13:01Z

Coverage increased (+0.04%) to 87.209% when pulling 0cbdfc3 on getaroom:add-calendar-icon into 0efa253 on airbnb:master.

AntiFish03 · 2017-01-24T01:26:10Z

@ljharb thanks

majapw

Looks reasonable to me, although we'll want to do some work in the future to a11y this probs.

majapw · 2017-01-25T23:29:25Z

src/components/DateRangePickerInput.jsx

    return (
      <div
        className={cx('DateRangePickerInput', {
          'DateRangePickerInput--disabled': disabled,
        })}
      >
+        {(showDefaultInputIcon || customInputIcon !== null) &&


nit this could just be (showDefaultInputIcon || customInputIcon)

coveralls · 2017-01-25T23:38:27Z

Changes Unknown when pulling 2077332 on getaroom:add-calendar-icon into ** on airbnb:master**.

coveralls · 2017-01-25T23:55:38Z

Coverage increased (+0.04%) to 84.857% when pulling 2077332 on getaroom:add-calendar-icon into 351eb5a on airbnb:master.

@cgriego

@cgriego requested the addition of a calendar icon to the date range picker. Simple boolean to enable, default is disabled. Interaction is the same as a focus of the start date. Rename `showCalendarIcon` to `showInputIcon` and add `customInputIcon` At the request of the project members I have added the prop `customInputIcon` and as it can now show more than just a calendar icon, it was sensible to rename the prop to `showInputIcon`. Making requested changes. showInputIcon to showDefaultInputIcon. display default icon unless custom icon is given.

coveralls · 2017-01-26T00:29:55Z

Coverage increased (+0.04%) to 84.857% when pulling 2e1e6b8 on getaroom:add-calendar-icon into fa4d785 on airbnb:master.

AntiFish03 changed the title ~~Adding a calendar icon to the begining of the DateRangePickerInput~~ Adding an input icon to the begining of the DateRangePickerInput Dec 20, 2016

ljharb previously requested changes Dec 20, 2016

View reviewed changes

ljharb force-pushed the add-calendar-icon branch from 3c7b6d1 to 0cbdfc3 Compare January 23, 2017 06:00

majapw force-pushed the add-calendar-icon branch 2 times, most recently from 48623b6 to 2077332 Compare January 25, 2017 23:28

majapw approved these changes Jan 25, 2017

View reviewed changes

majapw force-pushed the add-calendar-icon branch from 2077332 to 2e1e6b8 Compare January 26, 2017 00:04

majapw merged commit 93173d8 into react-dates:master Jan 26, 2017

majapw added the semver-minor: new stuff Any feature or API addition. label Jan 26, 2017

JemarJones mentioned this pull request Jan 31, 2017

Option for input icon for end date #278

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding an input icon to the begining of the DateRangePickerInput #222

Adding an input icon to the begining of the DateRangePickerInput #222

AntiFish03 commented Dec 19, 2016

coveralls commented Dec 19, 2016

coveralls commented Dec 19, 2016

lencioni commented Dec 19, 2016

AntiFish03 commented Dec 19, 2016

majapw commented Dec 20, 2016

AntiFish03 commented Dec 20, 2016

AntiFish03 commented Dec 20, 2016

ljharb Dec 20, 2016

AntiFish03 Dec 20, 2016

ljharb Dec 21, 2016

AntiFish03 Dec 21, 2016

ljharb Dec 21, 2016 •

edited

Loading

AntiFish03 Dec 21, 2016 •

edited

Loading

AntiFish03 Dec 21, 2016

majapw Jan 12, 2017

AntiFish03 Jan 12, 2017

coveralls commented Dec 20, 2016

AntiFish03 commented Dec 20, 2016 •

edited

Loading

majapw commented Jan 12, 2017

AntiFish03 commented Jan 21, 2017

coveralls commented Jan 21, 2017

coveralls commented Jan 21, 2017

coveralls commented Jan 21, 2017

ljharb commented Jan 21, 2017

AntiFish03 commented Jan 23, 2017

ljharb commented Jan 23, 2017

coveralls commented Jan 23, 2017

AntiFish03 commented Jan 24, 2017

majapw left a comment

majapw Jan 25, 2017

coveralls commented Jan 25, 2017

coveralls commented Jan 25, 2017

coveralls commented Jan 26, 2017

Adding an input icon to the begining of the DateRangePickerInput #222

Adding an input icon to the begining of the DateRangePickerInput #222

Conversation

AntiFish03 commented Dec 19, 2016

coveralls commented Dec 19, 2016

coveralls commented Dec 19, 2016

lencioni commented Dec 19, 2016

AntiFish03 commented Dec 19, 2016

majapw commented Dec 20, 2016

AntiFish03 commented Dec 20, 2016

AntiFish03 commented Dec 20, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ljharb Dec 21, 2016 • edited Loading

Choose a reason for hiding this comment

AntiFish03 Dec 21, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Dec 20, 2016

AntiFish03 commented Dec 20, 2016 • edited Loading

majapw commented Jan 12, 2017

AntiFish03 commented Jan 21, 2017

coveralls commented Jan 21, 2017

coveralls commented Jan 21, 2017

coveralls commented Jan 21, 2017

ljharb commented Jan 21, 2017

AntiFish03 commented Jan 23, 2017

ljharb commented Jan 23, 2017

coveralls commented Jan 23, 2017

AntiFish03 commented Jan 24, 2017

majapw left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Jan 25, 2017

coveralls commented Jan 25, 2017

coveralls commented Jan 26, 2017

ljharb Dec 21, 2016 •

edited

Loading

AntiFish03 Dec 21, 2016 •

edited

Loading

AntiFish03 commented Dec 20, 2016 •

edited

Loading