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

issue #1530 - Fix date selection in the SDP #1578

Merged

Conversation

Projects
None yet
4 participants
@GattoLupesco
Copy link

GattoLupesco commented Mar 13, 2019

Fixes #1530

@coveralls

This comment has been minimized.

Copy link

coveralls commented Mar 13, 2019

Coverage Status

Coverage decreased (-0.2%) to 84.285% when pulling f58954b on GattoLupesco:fix_singledatepicker_focus_on_ie11 into bc5c736 on airbnb:master.

Show resolved Hide resolved src/components/SingleDatePicker.jsx Outdated
@jrparish

This comment has been minimized.

Copy link
Contributor

jrparish commented Apr 2, 2019

@GattoLupesco - do you need help with this at all? This is a critical fix and we'd love to have your changes as soon as possible.

@ljharb

This comment has been minimized.

Copy link
Member

ljharb commented Apr 2, 2019

@jrparish if you wanted to write tests, ping me here with a link to your branch and i can pull them into this PR.

@jrparish

This comment has been minimized.

Copy link
Contributor

jrparish commented Apr 2, 2019

@ljharb - I made some tweaks to match the logic to what is being done in the DateRangePicker and then added some tests -
Branch: https://github.com/jrparish/react-dates/tree/fix_singledatepicker_focus_on_ie11
Commit: jrparish@b75d56c

@ljharb ljharb force-pushed the GattoLupesco:fix_singledatepicker_focus_on_ie11 branch from 58c0c12 to f58954b Apr 7, 2019

@ljharb

ljharb approved these changes Apr 7, 2019

Copy link
Member

ljharb left a comment

LGTM if tests pass

@@ -36,7 +37,6 @@ module.exports = (config) => {
presets: ['airbnb'],
},
},
{ test: /\.json$/, loader: 'json-loader' },

This comment has been minimized.

Copy link
@ljharb

ljharb Apr 7, 2019

Member

why is this removed?

This comment has been minimized.

Copy link
@jrparish

jrparish Apr 8, 2019

Contributor

This was throwing an error when running karma and is not needed with webpack 4.

@ljharb

This comment has been minimized.

Copy link
Member

ljharb commented Apr 7, 2019

@jrparish Although the karma tests aren't required, they still failed here - is that something you'd be willing to take a look at? (in a followup PR would be fine too)

@ljharb

This comment has been minimized.

Copy link
Member

ljharb commented Apr 7, 2019

actually i'm going to merge this as-is, since it leaves the karma tests in a much better situation.

@ljharb ljharb merged commit 28ce831 into airbnb:master Apr 7, 2019

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.2%) to 84.285%
Details
@jrparish

This comment has been minimized.

Copy link
Contributor

jrparish commented Apr 8, 2019

I can take a look at the other karma tests in a follow up 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.