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

feat(datepicker): add config service to provide default values #677

Closed
wants to merge 1 commit into from

Conversation

jnizet
Copy link
Member

@jnizet jnizet commented Sep 4, 2016

refs #518

@icfantv
Copy link
Member

icfantv commented Sep 5, 2016

@jnizet needs rebasing

config.firstDayOfWeek = 2;
config.markDisabled = (date) => false;
config.minDate = {year: 2000, month: 0, day: 1};
config.maxDate = {year: 2030, month: 1, day: 31};
Copy link
Member

Choose a reason for hiding this comment

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

I don't think 31FEB is a valid day. :-)

Copy link
Member

@icfantv icfantv Sep 5, 2016

Choose a reason for hiding this comment

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

Do we want to hard code this as a value or can we calculate it as added time from the minDate? The reason I ask is because in other calendar systems, 01JAN is not the first of the year.

This may mean that we need to have a base config object and have different calendars have different extensions of the config - which I'm ok with. @pkozlowski-opensource @jnizet, thoughts?

If a base config is the way we decide to go, then we'd need a Gregorian config subclass.

Copy link
Member Author

Choose a reason for hiding this comment

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

If I read the code correctly:

By default, if left undefined, the min date is computed by using the 1st of January of ten years before the start date (or now if there is no start date).

By default, if left undefined, the max date is computed by using the 31 of December of ten years after the start date (or now if there is no start date).

@maxokorokov can you confirm?

But this is done by the Calendar abstraction, which by default is Gregorian, but can be changed to another implementation. So I think we're fine here.

Note that the above function is just used in unit tests to check that custom config values are honored by the component. I didn't change anything to the min/max date selection logic.

@icfantv
Copy link
Member

icfantv commented Sep 5, 2016

@jnizet I don't see any default values for the config. Was this intentional?

@jnizet
Copy link
Member Author

jnizet commented Sep 5, 2016

The config service uses the same default values as the original datepicker component. Except for the first day of week and for the boolean flags, the default value is undefined, because nothing is mark disabled by default, and the start, min and max dates are computed automatically from the current date if not explicitly specified.

@maxokorokov
Copy link
Member

Hey @jnizet, sorry for a late reply. On vacations here :)
But yes, I confirm your comment about the maxDate. Its the last day of the year in 10 years from today or start date.

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

Successfully merging this pull request may close these issues.

None yet

3 participants