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

Add readonly attribute to DateInput when it isn't typeable #547

Merged
merged 7 commits into from
Jul 20, 2018

Conversation

whossname
Copy link
Contributor

Adding this attribute removes the caret from the text input and improves the behaviour when the datepicker is used on your phone. The caret is still there for Firefox but it works for Chrome and Safari. On phones the readonly attribute stops the keyboard from appearing, which is annoying if the date picker isn't typeable.

I also fixed a test that breaks when run in the eastern hemisphere and fixed some issues the linter identified.

@coveralls
Copy link

coveralls commented Jun 28, 2018

Coverage Status

Coverage remained the same at 100.0% when pulling bd99d20 on Haultrax:master into 6b89010 on charliekassel:master.

@mr-bantsevich
Copy link

It's very useful for us and we wait this feature!

@@ -239,7 +239,7 @@ describe('Datepicker.vue set by timestamp', () => {
wrapper = shallow(Datepicker, {
propsData: {
format: 'yyyy MM dd',
value: new Date(2018, 0, 29).getTime()
value: new Date(Date.UTC(2018, 0, 29, 0, 0, 0)).getTime()
Copy link
Contributor

Choose a reason for hiding this comment

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

The 0, 0, 0 part is redundant

If hours is supplied, let h be ToNumber(hours); else let h be 0.
If minutes is supplied, let min be ToNumber(minutes); else let min be 0.
If seconds is supplied, let s be ToNumber(seconds); else let s be 0.
If ms is supplied, let milli be ToNumber(ms); else let milli be 0.

Hopefully this gets merged soon

@saintplay
Copy link
Contributor

@whossname
Copy link
Contributor Author

@saintplay I didn't even realise there was already an issue open for #497. I'm on holiday at the moment so I will add your feedback into the pull request when I get back.

@whossname
Copy link
Contributor Author

and check out #505 as well

@FabianMeul
Copy link

Came here looking for this. Great PR! Any idea on when this gets released? :-)

@whossname
Copy link
Contributor Author

@saintplay made the changes, could you please give it another review? What is the next step towards getting it merged into the main repository?

@saintplay
Copy link
Contributor

Looks good to me. @charliekassel

this.disabledDates.from &&
(this.utils.getMonth(date) > this.utils.getMonth(this.disabledDates.from) && this.utils.getFullYear(date) >= this.utils.getFullYear(this.disabledDates.from)) ||
(this.disabledDates.from &&
this.utils.getMonth(date) > this.utils.getMonth(this.disabledDates.from) &&
Copy link
Owner

Choose a reason for hiding this comment

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

What is this change of behaviour for?
In any case I don't think we need that this.disabledDates.from as it's handled in the line before.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The linter was complaining about mixing the && and || operators like that. I'm pretty sure this doesn't change any functionality?

Copy link
Owner

Choose a reason for hiding this comment

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

I've removed the extra check but now conflicts with your branch, can you pull and fix at your end please?

@whossname
Copy link
Contributor Author

@charliekassel I merged in the fix

@charliekassel charliekassel merged commit 37eabb2 into charliekassel:master Jul 20, 2018
@nickhall
Copy link

@charliekassel any idea when this will be up on npm? I'm working on something that uses this and really need to prevent the keyboard from popping up asap

@whossname
Copy link
Contributor Author

@nickhall in the mean time do the following to get it working for your project:

  1. edit your package.json dependencies. Change the line for vuejs-datepicker to the following:
    "vuejs-datepicker": "git://github.com/charliekassel/vuejs-datepicker.git"
  2. Install your dependencies using npm or yarn
  3. Change your directory to node_modules/vuejs-datepicker
  4. build vuejs-datepicker using the following command:
    yarn && yarn lint && rm -rf dist/* && babel-node scripts/build.js && babel-node scripts/build-locale.js
    It might be different if you are on Windows. I'm not sure how to do it if you don't use yarn.

@FabianMeul
Copy link

Kind of waiting for this to get published on NPM :-)
If you could find the time to publish this, it would be greatly appreciated!

@riksnelders
Copy link

Waiting for this. Mobile keyboard pop up stinks :)

@whossname
Copy link
Contributor Author

@riksnelders version 1.5.3 was published to NPM 3 days ago. I haven't tested it myself yet, but the new version should include this pull request.

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

Successfully merging this pull request may close these issues.

None yet

8 participants