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

Fixed #3143: fixed functionality of 'show-weeks' attribute on datepicker popup #3149

Closed
wants to merge 7 commits into from

Conversation

ankit5990
Copy link
Contributor

Show weeks attribute was not working on datepicker popup.
Even when its value is set to 'false', the calender still shows weeks as per issue #3143.

The fixed version can be seen on plunkr
http://plnkr.co/edit/MJE44Blp7D2gIKWBOuwv?p=preview

@karianna
Copy link
Contributor

@ankit5990 Merge conflict, can you rebase and push please.

@ankit5990
Copy link
Contributor Author

ya sure lemme fix that. Its a pretty old PR so conflicts must have crept in...

@ankit5990
Copy link
Contributor Author

Hi @karianna ,
I have resolved the conflicts, please try now.

@karianna karianna added this to the 0.13.0 milestone Mar 31, 2015
@chrisirhc
Copy link
Contributor

Needs a test.

@@ -529,6 +529,11 @@ function ($compile, $parse, $document, $position, dateFilter, dateParser, datepi
if (attrs.dateDisabled) {
datepickerEl.attr('date-disabled', 'dateDisabled({ date: date, mode: mode })');
}

if(attrs.showWeeks) {
datepickerEl.attr('show-weeks',attrs.showWeeks);
Copy link
Contributor

Choose a reason for hiding this comment

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

space after , and above, space before (;

@ankit5990
Copy link
Contributor Author

@chrisirhc, yes I'll make the corrections and submit the code again with the required tests. Gimme a couple of days. I'm not very proficient with testing framework...

@ankit5990
Copy link
Contributor Author

HI @chrisirhc, I have added the tests for the bug fix change. Please review. :)

@chrisirhc
Copy link
Contributor

Squashed and merged. Also cleaned up trailing spaces. Thank you!

@ankit5990
Copy link
Contributor Author

Thanks for merging the fix @chrisirhc :)

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

Successfully merging this pull request may close these issues.

3 participants