Skip to content
This repository has been archived by the owner on Jan 7, 2022. It is now read-only.

Adding Allowed Date Ranges #226

Open
wants to merge 33 commits into
base: v1.x
Choose a base branch
from

Conversation

phyzical
Copy link

  • started functionality to support allowed date ranges instead of a distinct min and max
  • Refactored min, max date into new 'allowedDateTimeRanges'
  • added some helper functions to simplify the isDisabled functionality

jack added 4 commits June 16, 2020 17:25
…istinct min and max

* Refactored min, max date into new 'allowedDateTimeRanges'
* added some helper functions to simplify the isDisabled functionality
i blame gin.
hours and minutes now work
@phyzical
Copy link
Author

phyzical commented Jun 17, 2020

hey @mariomka i know your a busy dude.

and i know this contains alot big of changes...
i am still working my way through reviewing what i have changed.

The purpose of this pr will be to provide a prop of "allowed datetime ranges", if it ends up being too game changing i will just use my fork within my companies projects once im happy with it.

ANYWAY how do i go about installing my fork via npm as when i try the usual method of "git+https://[email protected]/phyzical/vue-datetime#AddingDisabledDatesSupport" i seem to be missing a dist folder?

in the meantime i will just commit the build, and remove it when it comes time for a formal PR

@mariomka
Copy link
Owner

Hey, it seems to be very useful but I can't guarantee anything. Right now my time is very limited.

Anyway, ping me when the PR is ready. I will try to review and merge it, but please, add some tests.

jack added 3 commits June 17, 2020 17:01
* addeda. helper func to convert these
* started fixing tests Please enter the commit message for your changes. Lines starting
@lucas7793
Copy link

@phyzical Are you still working on this feature?
The idea looks really good to me, I'm looking for this feature, but haven't found any DateTimePicker component which allows this.

@phyzical
Copy link
Author

i am still working on it but its just about finding the time haha. (i have a week off between jobs next week so ill try to get it done then)

feel free to checkout the fork and give it a try.

Let me know if you run into any issues usability wise or functionality.

👍

@lucas7793
Copy link

Great! I'm now trying it in a project and it works as expected. This addition is very nice.
How do I specify min and max datetime as it was previously? If I only want to enable all days after the current time, or before today?

alanbosco and others added 3 commits August 20, 2020 01:59
* alanbosco-feature-backdrop:
  Added new props for and hide backdrop
  Added new props for backdrop-click
@phyzical
Copy link
Author

phyzical commented Aug 22, 2020

hey @lucas7793,

Thanks for trying it out 👍

so what i tried to do was make it as backward compatible as possible.

what i ended up leaning towards is if you dont provide any allowed date ranges then min and max SHOULD work as it did before.

i did consider just adding the min and max even if you added allowed date ranges. but the way i see this new prop is its just a bunch of "min max ranges", so if you needed allowed date ranges you could just make you own "min range"

if (min && max) {
      minMaxArray.push(min)
      minMaxArray.push(max)
    } else if (min) {
      minMaxArray.push(min)
      minMaxArray.push(DateTime.fromFormat('31-12-3000 23:59:59', 'dd-MM-y hh:mm:ss'))
    } else if (max) {
      minMaxArray.push(DateTime.fromFormat('01-01-1972 00:00:00', 'dd-MM-y hh:mm:ss'))
      minMaxArray.push(max)
}

But upon thinking about it maybe we should just add the min and/or max range even if allowed ranges are added?

i not sure though as maybe this would add uneeded complexity when configuring it

…the "visibilty" to a object property due to how the current logic was using it otherwise it would always think the am/pm was never available
@phyzical phyzical marked this pull request as ready for review August 22, 2020 11:08
jack and others added 9 commits August 22, 2020 23:22
* addeda. helper func to convert these
* started fixing tests Please enter the commit message for your changes. Lines starting
…the "visibilty" to a object property due to how the current logic was using it otherwise it would always think the am/pm was never available
… of github.com:phyzical/vue-datetime into AddingDisabledDatesSupport # Conflicts: # README.md
@phyzical
Copy link
Author

hey @lucas7793,

If you find a chance please give the latest a go i think this should do all that is needed, please note that min and max becomes redundant once applying allowed date ranges.

hey @mariomka,

Finally got around to fixing the tests. if you find time please have a read and let me know of anything that concerns you and ill update accordingly when i find time 👍

@lucas7793
Copy link

@phyzical Sorry, I was busy the last weeks.
Everything is working great, I really like that addition. I skipped using min/max and only use the date ranges.

@Powell-v2
Copy link

Heyyo, any estimate on merging this feature?

@mariomka
Copy link
Owner

Hi everyone, I was thinking about it and this solution only fixes some of many of the questions and requests about disabling specific dates. I think the best solution is to provide a function to determine when a day or time is enabled or not, something like:

<template>
  <datetime :disabledDates="disabledDates"></datetime>
</template>

<script>
export default {
  methods: {
    disabledDates (year, month, day, hour, minute) {
      return true;
    }
  }
}
</script>

It allows full customization for every scenario.

What do you think?

@phyzical
Copy link
Author

hey @mariomka hope you have been well!

I think that's actually a really good idea, avoids us missing/catering too much towards something functionality wise as it defers it onto the implementation side 👍

just incase i'm misinterpreting what your suggesting

using src/DatetimeMonthPicker.vue as an example

 before pr : disabled: !index || monthIsDisabled(this.minDate, this.maxDate, this.year, index)
 current pr: disabled: !index || monthIsDisabled(this.allowedDateTimeRangesFormatted, this.year, index)
 after:  !index || monthIsDisabled(this.minDate, this.maxDate, this.year, index) || disabledDateTime(this.year)

so ill be essentially reverting majority of the changes but instead add this new method prop which will essentially be configured implementation side to do what ive proposed in the pr 👍

ill try to get this done over the weekend. But i might instead create a new branch/pr as i think this will be alot smaller given the proposal 😄

@mariomka
Copy link
Owner

@phyzical yes, that is the idea!

And, yes, it's better to create a new pr I think.

I don't have a lot of time but if the PR only modifies essentials parts of code and it is tested I think I could review and merge it in a short time.

Thank you!

@lewiswilbynm
Copy link

lewiswilbynm commented May 21, 2021

Hi all,

@mariomka is it likely that this pull request will be merged any time soon?

@tripflex
Copy link

Just for reference, I did something similar in my own fork -- but more-so in my case around disabling specific dates and time ranges (open hours, etc), and for disabling holidays and specific days (monday, friday, etc):
https://github.com/tripflex/vue-datetime

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

Successfully merging this pull request may close these issues.

None yet

7 participants