Skip to content

Pass Date object for Action Input Value#1029

Merged
skjnldsv merged 1 commit intomasterfrom
fix/server#20520/format-confusion
Apr 27, 2020
Merged

Pass Date object for Action Input Value#1029
skjnldsv merged 1 commit intomasterfrom
fix/server#20520/format-confusion

Conversation

@gary-kim
Copy link
Copy Markdown
Contributor

@gary-kim gary-kim commented Apr 17, 2020

For addressing nextcloud/server#20520

vue2-datepicker expects a v-model with a Date but the value type checks for string only.

@gary-kim gary-kim added bug Something isn't working 3. to review Waiting for reviews labels Apr 17, 2020
Comment thread src/components/DatetimePicker/DatetimePicker.vue
@georgehrke
Copy link
Copy Markdown
Contributor

I'm not quite sure why this has to be done in nc/vue and not just in the files app where it's needed.
Many apps (including the Calendar) rely on supplying a Date object to vue2-datepicker.

@gary-kim gary-kim force-pushed the fix/server#20520/format-confusion branch from ec93a1b to 311c368 Compare April 17, 2020 13:04
@gary-kim gary-kim changed the title Pass Date object to vue2-datepicker Pass Date object for Action Input Value Apr 17, 2020
@gary-kim gary-kim requested a review from skjnldsv April 17, 2020 13:05
@skjnldsv
Copy link
Copy Markdown
Contributor

skjnldsv commented Apr 22, 2020

Closing then: nextcloud/server#20538

@skjnldsv skjnldsv closed this Apr 22, 2020
@gary-kim
Copy link
Copy Markdown
Contributor Author

I think we should still merge this since other apps pass in Date objects?

@georgehrke georgehrke reopened this Apr 22, 2020
Comment thread src/components/ActionInput/ActionInput.vue Outdated
@skjnldsv
Copy link
Copy Markdown
Contributor

skjnldsv commented Apr 22, 2020

I think we should still merge this since other apps pass in Date objects?

I disagree, this is up to the dev to properly pass the data required by the datepicker library
nextcloud/server#20538 (comment)
https://github.com/mengxiong10/vue2-datepicker#value-type
We're not overwriting the library, the goal is to just have a design wrapper and a few shortcuts at most

@skjnldsv
Copy link
Copy Markdown
Contributor

skjnldsv commented Apr 22, 2020

Ah right, I confused myself! This is the other pr! 🙄
Let's get this in of course!
We should comply to what the library accept then :)

https://github.com/mengxiong10/vue2-datepicker/blob/fb8a79fecaaae2a21ca127f0ecf5299a7481d632/src/date-picker.vue#L138

@georgehrke
Copy link
Copy Markdown
Contributor

@skjnldsv check the changes again. All the changes from date to string are gone :)

@gary-kim
Copy link
Copy Markdown
Contributor Author

If I'm reading this correctly, it looks like Date, Number, and String are accepted. Let me push that in.

https://github.com/mengxiong10/vue2-datepicker/blob/fb8a79fecaaae2a21ca127f0ecf5299a7481d632/src/date-picker.vue#L336-L343

Signed-off-by: Gary Kim <gary@garykim.dev>
@gary-kim gary-kim force-pushed the fix/server#20520/format-confusion branch from 311c368 to 4592bfd Compare April 24, 2020 14:44
@gary-kim gary-kim requested a review from georgehrke April 24, 2020 14:45
@skjnldsv skjnldsv merged commit 0838856 into master Apr 27, 2020
@skjnldsv skjnldsv deleted the fix/server#20520/format-confusion branch April 27, 2020 12:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review Waiting for reviews bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants