Skip to content
This repository has been archived by the owner on May 29, 2019. It is now read-only.

[Proposal] Datepicker directive #366

Closed
bekos opened this issue Apr 28, 2013 · 13 comments
Closed

[Proposal] Datepicker directive #366

bekos opened this issue Apr 28, 2013 · 13 comments

Comments

@bekos
Copy link
Contributor

bekos commented Apr 28, 2013

After submitting this one ( #358 ) I thought I would be half job done without this directive :)

Please have a look at: http://plnkr.co/edit/hYWDccX8fIeWSeg3LA5I?p=preview

@jhiemer
Copy link

jhiemer commented Apr 28, 2013

Hi bekos,
took a first look into the PR. Here are my points:

  • The button design should be optional. I personally prefer the Link design as it does not feel that crimped.
  • Perhaps scrolling through the months as I have done here: http://plnkr.co/edit/PQOWFO?p=preview
  • Week numbers would also a good feature I think. Can also be found here: http://plnkr.co/edit/PQOWFO?p=preview
  • It would be nice to have at least two features for the usage in a real world scenario:
    • minDate, the date from whereon the buttons/links are activated.
    • maxDate the date to whereon the buttons/links are activated

And a more general question. Do you plan to put this directive into popover?

@bekos
Copy link
Contributor Author

bekos commented Apr 28, 2013

Hello @jhiemer and thank you for the time spent on this one.

Firstly this is not a PR, just an initial draft to see reactions before proceeding :-)

  • I agree that the template is a subject for change. I used buttons because they can display better the different states (selected / disabled / default) and the hover effects, as long as we cannot use custom CSS rules.
  • I am not very keen with the mousescroll ability since I have not see another major implementation (jquery ui, yui etc) use this. It is not very difficult to add later.
  • I updated the plunker example to display and toggle week numbers in the calendar.
  • I have thought of this from the beggining and I surprise my self by saying this but I think this should not be built into the directive, since there would be many different options to what features like this we shall support. For example you want minDate/maxDate, someone else may want Saturdays and Sundays to be disabled, someone else the last five days of each month to be disabled etc. My opinion is to provide an optional expression isSelectable which executes for each displayed date and returns true/false whether this one should be clickable or not, but I want to listen and what others @angular-ui/bootstrap have to say.

This directive displays inline of the parent's context. If you place it inside the body of a popover then it will display on the opening of the popover. In the same way you can display it and as body of a dropdown.

@jhiemer
Copy link

jhiemer commented Apr 28, 2013

Hi @bekos ,

  • Templates: ok understood. You are right this can be changed later, or even extracted into a template anyway, so each one can decide on its own, whether to use buttons, or links.
  • Mousescroll: I am fine with that.
  • Week numbers: great
  • MinMaxDate: ok got your issue. I see your point that it could get ugly. The question is, if we could plug in some generic filter interface, which gets applied to the number of dates, if the filter ist provided.

What do you think?

  • Display: Ok then we need to push the template url stuff of @joshdmiller in the popover issue so we can use it as a generic basis.

@pkozlowski-opensource
Copy link
Member

@bekos wow, this looks amazing! Holly cow, I can't believe it is all < 200 LOC. And even less if you extract a template. And if you extract a template it becomes fully customizable.

This is truly amazing, I knew that we are going to get here but still it is incredible to see this happening. I must say that this is the first fully customizable date picker I've ever seen (!).

Awesome job!

@bekos
Copy link
Contributor Author

bekos commented Apr 28, 2013

@jhiemer Good idea. Probably we can provide some default $services that take care of comparing dates, months, days of week etc. I think it's better to do it in second phase, after the initial implementation is approved.

@pkozlowski-opensource Thank you! and I hope you have seen enough date pickers :-)

@ajoslin
Copy link
Contributor

ajoslin commented Apr 28, 2013

Yay Tasos! You rock :-)

This looks awesome 👯 🍻 💯

@bekos
Copy link
Contributor Author

bekos commented Apr 28, 2013

Thank you very much Andy!

🎸 ✌️ 🍰(I couldn't resist) :-)

@bekos
Copy link
Contributor Author

bekos commented Apr 29, 2013

Updated example. Added starting-day attribute (default: 0 - I think there is no way at this moment to use $locale to set default value, no?) with possible values [0, 6], in order to be able to customize the first day of the week.

I guess, I shall open a PR now.

@jhiemer
Copy link

jhiemer commented Apr 29, 2013

@bekos that's the reason I was using moment.js in my implementation. The thing is, in any case we want to support localisation, we have to options

  • Rebuild anything from scratch with a large overhead regarding localisation handling etc.
  • Or use this 5KB tiny library called moment.js which offers all this stuff from the beginning on.

As much as I appreciate the abandonment of jQuery and co. the question for me here is, if moment.js could not be a suitable exception, reducing the amount of additionally required code.

@bekos
Copy link
Contributor Author

bekos commented Apr 29, 2013

@jhiemer AngularJS already supports localization and datepicker itself uses date $filter to render the appropriate localized labels for months, week days etc. If I am not wrong, I have seen the FIRSTDAYOFWEEK property in the i18n source files, but this is not passed in the $locale. Probably this is an issue for AngularJS.

@jhiemer
Copy link

jhiemer commented Apr 29, 2013

@bekos thanks, did not know that up to now. Everyday I learn something new... :-)

@mikehaas763
Copy link

This is awesome @bekos. I submitted #315 a while back and was just about to begin work on it this weekend. No longer needed! Thank you!

Do you have anything you would like me to help with revolving around this directive?

@bekos
Copy link
Contributor Author

bekos commented May 1, 2013

Thx @mikehaas763 :-)

Right now I'm trying to write the tests, fix bug with week numbering and make it a bit more optimized by not watching many things & using less scope functions in the template. I hope to make a PR in the next days.

A code review & testing will be very much appreciated!

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

No branches or pull requests

5 participants