This repository has been archived by the owner on May 29, 2019. It is now read-only.
Datepicker refactor #1120
Closed
Datepicker refactor #1120
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Finally I found a way to make it work properly... Now it doesn't break any previous functionality but accepts also min-date and max-date attributes. I also added some tests to verify it's working, because before I did this, it wasn't possible to use the show-weeks attribute in the popup |
|
keep the good work up |
|
@bekos any thought about this? |
|
Hi @ivogallego and sorry for not responding earlier. Closing because datepicker is under refactor (#1599) and this cannot be merged atm. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a PR just for discussion, so don't expect it to be fully functional...
What I'm doing here, is trying to unify somehow all the configuration, because right now some of the config fields accessable from the datepicker directive aren't from the popup.
I've changed the the min and max attributes for their real name in the config file. I guess you already discussed about this, but as user of the library, I found it a mess having to know how to call the same parameter depending on where I'm calling it from.
As you can see, I did this functionality kind of global in order to share it as well with the datepicker directive, but the problem I'm currently having is that I don't know how to process the different behavior of each of the parameters in an elegant way... I'm a java/groovy developer who recently moved to Angular and JS, so I'm starting to read some books about JS and see if I kind find a pattern, or at least an idea of how could I do this... Any idea, even if you don't like the whole approach would be much more than welcome! :)
There's another thing I'd like to point out and is the datepickerOptions. We need to specify the properties in the attribute way (hyphen separated), but the 'Angular' way is to use camel-case format in JS and hyphen separated format for html, so I think is worth to convert the popup configuration before add it to the datepicker element in order to have consistency along all the APIs.
I know maybe there are a lot of breaking changes, but I always love consistency and I think this is an awesome library.which really deserves all my efforts.
P.D: sorry for doing this PR so late, but I've bee really busy this week and I wanted to explain myself in the best way I'm able to...