mattlewis92 / angular-bootstrap-calendar

A port of the bootstrap calendar widget to AngularJS (no jQuery required!)

https://mattlewis92.github.io/angular-bootstrap-calendar/

MIT License

798 stars 369 forks source link

feat(excluded-days): Allow users to specify a set of days to be excluded from month and week view #640

Closed santam85 closed 7 years ago

santam85 commented 7 years ago

Taking inspiration from #570, I modified the directives to add an "excluded-days" attribute, that allows the user to specify which weekdays to include in month and week views. Date modifier also supports it for day view, optionally skipping some days while navigating.

codecov-io commented 7 years ago

Codecov Report

Merging #640 into master will increase coverage by 0.05%. The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #640      +/-   ##
==========================================
+ Coverage   95.94%   95.99%   +0.05%     
==========================================
  Files          26       26              
  Lines         715      724       +9     
==========================================
+ Hits          686      695       +9     
  Misses         29       29

Impacted Files	Coverage Δ
src/services/calendarHelper.js	`100% <100%> (ø)`	:arrow_up:
src/directives/mwlCalendarMonth.js	`94.66% <100%> (ø)`	:arrow_up:
src/directives/mwlDateModifier.js	`100% <100%> (ø)`	:arrow_up:
src/directives/mwlCalendar.js	`92.72% <100%> (+0.13%)`	:arrow_up:
src/directives/mwlCalendarWeek.js	`100% <100%> (ø)`	:arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 1678ae7...0db5db0. Read the comment docs.

mattlewis92 commented 7 years ago

This is absolutely perfect, nice job! I just want to clone it first and physically play around with it, but I will try and merge it asap 👍

santam85 commented 7 years ago

@mattlewis92 thanks! I am working on improving the coverage for those lines I added to the dateModifier, might want to wait for that!

santam85 commented 7 years ago

@mattlewis92 All done!

santam85 commented 7 years ago

@mattlewis92 I guessed I rushed it a bit too much...all should be fixed.

The resizing and dragging issue was because of the fact that the column size is now dynamic based on the number of days, and it is caused by the added ng-class on the weekview template. By moving the elementDimensions directive on the entire row, and calculating the column size based on that, It should work correctly. Moreover, the directive triggers just once now, so it's also a nice improvement. I tried to keep template changes to a minimum otherwise.

santam85 commented 7 years ago

@mattlewis92 I performed the required changes, looks like the bugs are gone. Have a look when you have time! Let me know if there are other changes required...

mattlewis92 commented 7 years ago

@santam85 sorry about the late reply on this, been super busy with work and real life lately, not had much time for open source. I've just spotted a regression in the year view, would you mind taking a look when you have a sec? Thanks!

With this PR:

Master:

santam85 commented 7 years ago

@mattlewis92 no worries, I was just a bit impatient to use the new feature on my project! Will have a look ASAP!

igler commented 7 years ago

I'd love to see this in master-branch as I need this feature in one of my projects. Any chance to release this in the near future?

mattlewis92 commented 7 years ago

@igler it's just pending https://github.com/mattlewis92/angular-bootstrap-calendar/pull/640#issuecomment-325353749 to be fixed, you can point your bower / npm to the branch though if you want to use it today

igler commented 7 years ago

You mean the development branch?

mattlewis92 commented 7 years ago

Sorry I meant the fork that this PR is based on: santam85/angular-bootstrap-calendar#feature/excluded-days

santam85 commented 7 years ago

@mattlewis92 regression fixed, did you find anything else?

mattlewis92 commented 7 years ago

I think that was it, I'm AFK today, but will try and get this merged tomorrow. Thanks a lot for your contribution! 😀

santam85 commented 7 years ago

@mattlewis92 I'm sorry to bother you, but any news on this?

mattlewis92 commented 7 years ago

Oops, sorry I completely forgot about this. I have just published this branch to npm as 0.30.0-beta.0, I'm just going to test it on our app at work to make sure there are no major regressions and then I'll merge it and release properly 😄

mattlewis92 commented 7 years ago

Published as 0.30.0, thanks for your patience on this 👍