Add customCloseIcon prop #356

moonboots · 2017-03-06T18:53:02Z

Adds a customCloseIcon prop to allow overriding the default <CloseButton />. I matched the behavior of customArrowIcon: default prop of null, fallback to original component inside render, and a wrapping css class.

coveralls · 2017-03-06T19:08:35Z

Coverage decreased (-5.04%) to 81.802% when pulling 48e136c on moonboots:custom-close-icon into 04d08d4 on airbnb:master.

majapw · 2017-03-06T23:23:19Z

src/components/DateRangePicker.jsx

@@ -273,7 +277,9 @@ export default class DateRangePicker extends React.Component {
            <span className="screen-reader-only">
              {this.props.phrases.closeDatePicker}
            </span>
-            <CloseButton />
+            <div className="DateRangePickerInput__close">


This class name is wrong. You're not inside the input.

majapw · 2017-03-06T23:25:42Z

stories/DateRangePicker_input.js

+      padding: '3px',
+    }}
+  >
+    {'X'}


This can just be X no?

majapw · 2017-03-06T23:26:33Z

test/components/DateRangePickerInput_spec.jsx

+          showClearDates
+          customCloseIcon={<span className="custom-close-icon" />}
+        />);
+      expect(wrapper.find('.DateRangePickerInput__calendar-icon .custom-close-icon'));


This should fail... why would it be under .DateRangePickerInput__calendar-icon? Why isn't this failing?

majapw

I'm concerned about the test, plus we need parallel tests for the SingleDatePicker components.

coveralls · 2017-03-07T00:16:05Z

Coverage increased (+0.3%) to 82.318% when pulling 09fd248 on moonboots:custom-close-icon into 43badcf on airbnb:master.

coveralls · 2017-03-07T01:27:16Z

Coverage increased (+0.1%) to 82.318% when pulling 3f95071 on moonboots:custom-close-icon into 9704b74 on airbnb:master.

coveralls · 2017-03-07T01:55:48Z

Coverage increased (+0.1%) to 82.318% when pulling 3f95071 on moonboots:custom-close-icon into 9704b74 on airbnb:master.

coveralls · 2017-03-07T02:02:37Z

Coverage increased (+0.1%) to 82.318% when pulling 3f95071 on moonboots:custom-close-icon into 9704b74 on airbnb:master.

coveralls · 2017-03-07T19:39:22Z

Coverage increased (+0.1%) to 82.146% when pulling 0a31762 on moonboots:custom-close-icon into 130e542 on airbnb:master.

moonboots · 2017-03-07T19:41:26Z

@majapw I've added a corresponding spec for SingleDatePicker. The spec you pointed out wasn't failing because it accidentally didn't have any assertions, only a dangling expect(..);, fixed now.

majapw · 2017-03-09T17:04:46Z

test/components/SingleDatePickerInput_spec.jsx

+            showClearDates
+            customCloseIcon={<span className="custom-close-icon" />}
+          />);
+        expect(wrapper.find('.DateRangePickerInput__calendar-icon .custom-close-icon'));


You now have the same problem here in this spec...

Oops, fixed

coveralls · 2017-03-09T18:26:47Z

Coverage increased (+0.1%) to 82.146% when pulling 9f18afd on moonboots:custom-close-icon into 130e542 on airbnb:master.

coveralls · 2017-03-09T18:46:23Z

Coverage increased (+0.1%) to 82.146% when pulling 9f18afd on moonboots:custom-close-icon into 130e542 on airbnb:master.

zeemawn · 2017-03-21T08:46:05Z

Correct me if I'm wrong, but shouldn't this whole thing rather be called "custom CLEAR icon"?
The 'X' might look like a close-icon, but its function is to clear the dates, isn't it?
(Besides that, it would be nice to have an actual close-button)

ljharb · 2017-03-21T18:22:53Z

It closes the datepicker, it doesn't clear the dates.

majapw · 2017-03-21T18:29:30Z

It actually does both... We overload the icon in use, so uh, I get the Q

ljharb · 2017-03-21T18:36:28Z

Lol well there you go

ljharb · 2017-03-23T17:46:36Z

@majapw can we unoverload them?

interactivejohn · 2018-04-09T15:58:00Z

I ran into the same issue as @zeemawn. I spent a good couple hours thinking that I could add a custom close icon to the default state, only to find out that it was only implemented for the showClearDate and withFullScreenPortal props. Maybe something can be added to the README that explains where the customCloseIcon is implemented? Meanwhile I will fork this package and start working on implementing a showDefaultCloseIcon for the DatePicker as requested above (I also need it for a project).

majapw reviewed Mar 6, 2017

View reviewed changes

stories/DateRangePicker_input.js Outdated

padding: '3px',

}}

>

{'X'}

Copy link

Collaborator

majapw Mar 6, 2017

Choose a reason for hiding this comment

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

This can just be X no?

majapw reviewed Mar 6, 2017

View reviewed changes

majapw requested changes Mar 6, 2017

View reviewed changes

moonboots force-pushed the custom-close-icon branch from 48e136c to 09fd248 Compare March 6, 2017 23:28

moonboots force-pushed the custom-close-icon branch 3 times, most recently from f30d5fb to 3f95071 Compare March 7, 2017 01:06

majapw reviewed Mar 9, 2017

View reviewed changes

Add customCloseIcon prop

6d45150

moonboots force-pushed the custom-close-icon branch from 0a31762 to 6d45150 Compare March 9, 2017 18:12

Merge branch 'master' into custom-close-icon

9f18afd

majapw approved these changes Mar 9, 2017

View reviewed changes

majapw merged commit eab9c04 into react-dates:master Mar 9, 2017

moonboots deleted the custom-close-icon branch March 9, 2017 19:08

ljharb mentioned this pull request Apr 12, 2019

Doesn't work on IE11 #1076

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add customCloseIcon prop #356

Add customCloseIcon prop #356

moonboots commented Mar 6, 2017

coveralls commented Mar 6, 2017

majapw Mar 6, 2017

majapw Mar 6, 2017

majapw Mar 6, 2017

majapw left a comment

coveralls commented Mar 7, 2017

coveralls commented Mar 7, 2017

coveralls commented Mar 7, 2017

coveralls commented Mar 7, 2017

coveralls commented Mar 7, 2017

moonboots commented Mar 7, 2017

majapw Mar 9, 2017

moonboots Mar 9, 2017

coveralls commented Mar 9, 2017

coveralls commented Mar 9, 2017

zeemawn commented Mar 21, 2017

ljharb commented Mar 21, 2017

majapw commented Mar 21, 2017

ljharb commented Mar 21, 2017

ljharb commented Mar 23, 2017

interactivejohn commented Apr 9, 2018 •

edited

Loading

Add customCloseIcon prop #356

Add customCloseIcon prop #356

Conversation

moonboots commented Mar 6, 2017

coveralls commented Mar 6, 2017

majapw Mar 6, 2017

Choose a reason for hiding this comment

majapw Mar 6, 2017

Choose a reason for hiding this comment

majapw Mar 6, 2017

Choose a reason for hiding this comment

majapw left a comment

Choose a reason for hiding this comment

coveralls commented Mar 7, 2017

coveralls commented Mar 7, 2017

coveralls commented Mar 7, 2017

coveralls commented Mar 7, 2017

coveralls commented Mar 7, 2017

moonboots commented Mar 7, 2017

majapw Mar 9, 2017

Choose a reason for hiding this comment

moonboots Mar 9, 2017

Choose a reason for hiding this comment

coveralls commented Mar 9, 2017

coveralls commented Mar 9, 2017

zeemawn commented Mar 21, 2017

ljharb commented Mar 21, 2017

majapw commented Mar 21, 2017

ljharb commented Mar 21, 2017

ljharb commented Mar 23, 2017

interactivejohn commented Apr 9, 2018 • edited Loading

interactivejohn commented Apr 9, 2018 •

edited

Loading