wet-boew / wet-boew

Web Experience Toolkit (WET): Open source code library for building innovative websites that are accessible, usable, interoperable, mobile-friendly and multilingual. This collaborative open source project is led by the Government of Canada.
https://wet-boew.github.io/wet-boew/index-en.html
Other
1.61k stars 661 forks source link

Input type="date" polyfill (date picker): Setting "min" as future date causes picker to open up at max date #8248

Open WalidBounouar opened 6 years ago

WalidBounouar commented 6 years ago

Hi,

I want to set the "min" attribute as a future date, but doing so causes the picker to open at the max date (if not set, opens at year 2100 which I believe is default max). However, the dates are disabled correctly if you navigate to them.

This happens only on some browsers. The native chrome picker is fine. I encounter the issue on Firefox 52.5 and IE 11.

Here’s code when inspecting page and screenshots. <input id="frmCCI-completion_date" name="frmCCI-completion_date" min="2018-01-11" size="10" type="date" class="form-control" />

imageimage

I believe we are using wet v4.0.26 where I am.

I looked in issues and did not find the same problem flagged before. Do you have an idea on what the problem is and how to fix it?

Thanks

ghost commented 6 years ago

I was able to reproduce with latest version of WET on Safari also (using the polyfill). When MIN is future dated the calendar opens in the year 2100.

ghost commented 6 years ago

Seem to be related to the following if statement in the polyfill:

if ( today >= state.minDate && today <= state.maxDate ) {
 initDate = today;
} else {
 initDate = state.maxDate;
}

Code can be found at datepicker.js#L120

WalidBounouar commented 6 years ago

Do you have an idea when this could be fixed? Would it be included in a potential v4.0.28 release or could it be added to v4.0.27?

duboisp commented 6 years ago

@WalidBounouar we can't provide a time estimate of when this issue would be fixed. However, you or someone else can submit a PR with a fix. The submission date of that PR with the necessary time to review it and having the contributor to fix any-reported issue would determinate which version of WET it will be included.

FYI - The version 4.0.27 is already released.

ghost commented 6 years ago

I don't mind submitting the PR. Do you think we can just change it too:

if ( today >= state.minDate && today <= state.maxDate ) {
 initDate = today;
} else {
 initDate = state.minDate; ---> FROM MAX TOO MIN?
}
LaurentGoderre commented 6 years ago

@duboisp @kodecount the calendar polyfill has unit tests. A failing unit test need to be created before the fix.

WalidBounouar commented 6 years ago

I am in talks with my superiors to check if I can take time to find a fix and submit a PR. I will keep tabs on this issue in case somebody else submits a fix (and unit testing). I'm new to this so I would also be unable to provide a time frame.

WalidBounouar commented 6 years ago

We decided to use a workaround where I am, so I won't be able to find a fix. I will still follow the progress if this issue. Thanks

duboisp commented 6 years ago

The behavior need to be conform to the HTML 5.3 spec.

I quickly reviewed and I didn't saw a mention about the default behavior when opening with a min date that is in the future.

Next step would be:

ghost commented 6 years ago

FireFox v58: opens the calendar in the current month. Chrome v68: opens the calendar based on the minimum set. jQuery Date Picker: opens the calendar based on the minimum set.

LaurentGoderre commented 6 years ago

The minimum date should be the right behavior.

ghost commented 6 years ago

Any documents on the unit test you want or is that just a working example?

LaurentGoderre commented 6 years ago

basically there should be two unit tests for when the minimum date is in the future and when the maximum date is in the past that resembles this one:

https://github.com/wet-boew/wet-boew/blob/master/src/polyfills/datepicker/test.js#L477-L499

WalidBounouar commented 6 years ago

Hi, I would like to know if this issue number is sufficient to track progress on this bug? I don't know if you have another bug tracking system at Treasury Board.

We would like to be able to see when this is fixed so that we can move to a new release of WET.

duboisp commented 6 years ago

@WalidBounouar yes this issue number should be sufficient to track progress on this bug.

If having this issue fixed matter for you or for your organisation, the best option will be to spent your time on resolving it and submitting a PR. If you can work on a fix and need help, contact me and we can have a quick chat. You can find my info on GCconnex, on GCcollab or you can reach me via twitter.

WalidBounouar commented 6 years ago

@duboisp thanks for the speedy reply.

If I can get the time to find a fix for this, I'll mention it here. You're contact info is very much appreciated.

ghost commented 6 years ago

I just submitted a PR, but unsure if it need a unit test in the commit.