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 scope to customPredictor #813

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

lsapan
Copy link

@lsapan lsapan commented Apr 3, 2020

Hello, thanks for making this awesome library!

I realized as I was using it that when you disable dates using a customPredictor, that certain months and even years may become disabled. This is because the plugin checks the first date to see if it's disabled, rather than checking if any day in the given month/year is enabled. Makes total sense from a performance perspective in case the computation is heavy!

What I've done is simply added a second parameter to customPredictor which lets us know what scope we're dealing with. So, let's say I wanted to only allow certain days of the week, I can ensure that the scope is day before I say to disable the other dates. If the scope is anything else I can just always allow it.

@coveralls
Copy link

coveralls commented Apr 3, 2020

Coverage Status

Coverage remained the same at 100.0% when pulling 6a910cf on lsapan:master into de0fc75 on charliekassel:master.

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.

2 participants