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

pickadate appears disabled in bootstrap 3 #474

Closed
acornejo opened this issue Jul 2, 2014 · 16 comments
Closed

pickadate appears disabled in bootstrap 3 #474

acornejo opened this issue Jul 2, 2014 · 16 comments
Labels

Comments

@acornejo
Copy link

acornejo commented Jul 2, 2014

Given the ubiquity of bootstrap, it would be great if pickadate could work around it.

Namely, by setting the readOnly flag on the input element, bootstrap styles the element as being disabled (i.e. not editable). This is confusing for the users, since they are led to believe that the date cannot be changed (and in fact, it can!).

Setting the "editable" property fixes this problem, but creates another (now the field can be edited with the keyboard). A better fix would be to disable input of the field via javascript instead of using the css property.

Using readOnly on the CSS is misleading, since the input field is NOT readOnly, it can be changed, except that the change will be performed through the pickadate UI and not by typing.

PS: This issue is related to #443, except that I am reporting a problem on usability, where the reporter of #443 was instead trying to figure out how to disable "for real" the input.

@davidimoore
Copy link

I found that this happens on the time picker but not the datepicker. For the time being I'm just changing the cursor...

@acornejo
Copy link
Author

acornejo commented Jul 3, 2014

@davidimoore I find that extremely strange, here is a jsfiddle illustrating the issue. As you can see when either datepicker or timepicker are enabled on a field, the fields appear as disabled in bootstrap.

http://jsfiddle.net/V9Skj/

This behavior can be traced to the readonly attribute, since bootstrap renders readonly input fields as disabled. This makes sense, since in theory you should not be able to interact/modify a field which is marked as readonly.

@davidimoore
Copy link

A have a it more of a complicated set up. I have both running on the same page. I also created a toggle effect for a date range versus one date.

If you scroll down to the Date and Time fieldset of my site you'll see what I mean:

http://www.eventfetch.com/promotion_requests/new

@davidimoore
Copy link

Still for the timepicker I had to override the cursor.

@acornejo
Copy link
Author

acornejo commented Jul 3, 2014

Thanks for taking the time to reply, ironically, in your example the issue does not appear on the datepicker because you did not use the correct CSS recommended by bootstrap on input elements.

Specifically you forgot to add the form-control class to the input element, and therefore it does not get styled correctly (or incorrectly in this case). Styling the cursor differently is certainly a step in the right direction, but I do believe a correct solution is not to use the readonly attribute.

@davidimoore
Copy link

Thanks for the catch. I was using an old partial. Just deployed that a few minutes ago. Can the read only attribute be removed and still allow for the click event to trigger the pop up? Maybe I'll hack at it later.

@davidimoore
Copy link

Just added the form control class and no change in behavior.

@davidimoore
Copy link

I take that back.

@acornejo
Copy link
Author

acornejo commented Jul 3, 2014

To answer your first question, yes you can remove it and "still allow for the click event to trigger the pop up". An ever better way to do this, is to set the editable property to true when initializing the picker, i.e. $(element).pickadate({editable: true});. This prevents pickadate from adding the readonly attribute on the first place. However, that is not a good solution, since the field can now be edited using the keyboard.

To answer your later comment, at least in the version deployed on the web you have NOT added the form-control class to the date input element. If I add it myself using chrome inspector I can see how the input element is styled as being disabled (and this is not a change in behavior per se, just a change in styling). (PS: just saw your other comment, so ignore this paragraph)

There is no clean solution without patching pickadate (hence why I opened this issue). If you are looking for clean solution without changing pickadate, I think the approach of styling the cursor differently is the best one (you could also add another class and override bootstrap's styling, but all of these are hacks).

@davidimoore
Copy link

My site hadn't loaded yet. Yes I can confirm that adding the form control creates the effect.

Overriding the styling for this narrow purpose doesn't feel too hacky.

@acornejo
Copy link
Author

acornejo commented Jul 3, 2014

I said it was a hack because its still working around the fact that the field is marked as readonly but is NOT actually readonly. (its like marking a checkbox as readonly only because it can't be toggled using the keyboard, and only with a mouse). Furthermore, I think its an issue worth addressing for the following reasons:

  1. Bootstrap is the most widely used CSS framework by far, so its nice for things to "just work" with it, instead of having to override styles for each plugin you use.
  2. A few other frameworks style input elements with the readonly attribute differently (and why shouldn't they? if a field is really readonly and can't be edited, it makes sense to style it differently).
  3. People with disabilities that use screen readers will also run into problems, since this input element is marked as read only.

However I agree that it is trivial for each of person that uses pickadate+boostrap to solve it on their own (I already created styles to override the readonly before posting this issue). I still opened this issue to keep track of this bug/missing-feature, but if you feel it doesn't merit attention feel free to close it.

@davidimoore
Copy link

I think it's definitely an issue worth addressing.
On the upside the calendar is still accessible when you tab to it, so some accessibility issues are addressed. I'm not sure however, how a screen reader would translate it.

Not to get too far off topic but I tend to override and exclude much of bootstrap's styling the more plugins that I add. I think Bootstrap is a good starting point, mostly for responsive layout. I don't like a lot of what's included. I also don' t like to be too confined. Still if you have a team working on a project without a good style guide one off overrides can create some havoc.

Thanks for posting this bug report.

@amsul
Copy link
Owner

amsul commented Jul 21, 2014

@acornejo I agree, it’s not actually readonly – unfortunately that’s the only way to prevent a touch device’s virtual keyboard from popping up.

This is the quick fix to the issue you’re experiencing with Bootstrap: http://jsfiddle.net/P3s3h/

I’m not sure if there’s any other way to address this.

@acornejo
Copy link
Author

Thanks @amsul

That is pretty close to the CSS patch I implemented, it definitely fixes the visual aspect of the problem.

I tried a JS solution where I manually catch the events and deal with it. Unfortunately you are right, in mobile devices the virtual keyboard still shows up for a split second before disappearing.

This may be one of those rare cases where there is no good solution (either in pickadate.js or bootstrap) and we have to settle for a minor hack.

@amsul
Copy link
Owner

amsul commented Oct 4, 2014

@acornejo yup. And just to put any accessibility concerns to rest, the input element does have aria-readonly="false".

I believe this is safe to mark as resolved with this CSS workaround.

Cheers!

@wilsonball
Copy link

@amsul Is there an update to your jsfiddle code? http://jsfiddle.net/P3s3h/ Not working.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants