Skip to content
This repository has been archived by the owner. It is now read-only.

feat(week-view): 5 days semi-weeks #570

Closed
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@azachar
Copy link

azachar commented Apr 3, 2017

Hello,
I have implemented a new feature that allows you to show only part of the week.
image

It would be great if you could help me to review the new code.

Also, I am not sure how to fix one failing test ( to be honest that view I do not need, so I left the investigation over). Perhaps you could have an idea how to fix it. I guess it would be for you super easy.

Anyway, thanks for your feedback!

Cheers,
Andrej

}
.cal-row-fluid .cal-cell6 {
width: 85.71428571428571%;
*width: 85.66098081023453%;

This comment has been minimized.

@azachar

azachar Apr 3, 2017

Author

From where this constant come from? THX.

@@ -0,0 +1,33 @@
.gridForWeekOf(@days) {
@oneDay: 100% / @days;
@oneDayIE: 0.9993781095 * @oneDay;

This comment has been minimized.

@azachar

azachar Apr 3, 2017

Author

I hope I set the constant according to your manual calculations.

@mattlewis92

This comment has been minimized.

Copy link
Owner

mattlewis92 commented Apr 3, 2017

Thanks this looks good! I don't have time to properly review it right now, however there are a couple of things I can see that you can work on now:

  • please remove the dist files from the commit, these get generated automatically on release
  • please can you remove the event editor from the new demo - I prefer to keep feature demos focused on just that particular feature
  • the calendar-utils lib already supports excluding days from the week view - you can pass an array of day indices to hide from the week view e.g. getWeekView({..., excluded: [0, 6]}) will remove all weekends from the view - please can you change the API to follow this approach instead of just passing a count of days to exclude - this way people will be able to choose the days they want to hide and leads to a more flexible API. You can see an example of how this works for the angular2 version of this calendar here
@azachar

This comment has been minimized.

Copy link
Author

azachar commented Apr 3, 2017

Hey @mattlewis92,

Thanks for your message.

ad 1) I need to somehow used this feature within my project, so until we merge this PR, will keep it there. (but yes I agree with you:)

ad 2) Sure, will remove it. (was there just temporary)

ad 3) calendar-utils support excluding, that is great, but it is a quite a messy to use it for this feature (initially I used but the algorithm to choose excluded days became very smelly).
Since my feature is not about hiding, it is about making the calendar available on devices that have a small screen (like a mobile phone for instance). So one semi week of 3 days starts for example on T(uesday), then continue to W(ednesay) and finish up with F(riday).
The next semi week is S,S,M, then T,W,F.. and still, it makes sense to filter out weekends.

So, I wound prefer to keep approaches transparent and not to reuse them. They have in my opinion different meanings. Do you see my point? And also, this should be in theory faster (instead of excluding, we are just iterating less days..)

Anyway, look forward hearing from you!

Cheers,
Andrej

@azachar azachar force-pushed the azachar:master branch from 654b761 to 5201c42 Apr 3, 2017

@azachar

This comment has been minimized.

Copy link
Author

azachar commented Apr 3, 2017

ad 2) removed :)

@DarshanVelagaleti

This comment has been minimized.

Copy link

DarshanVelagaleti commented Jun 27, 2017

can any one please help me to Hide Previous dates and after 60 days Dates on Calander

@mattlewis92

This comment has been minimized.

Copy link
Owner

mattlewis92 commented Sep 7, 2017

Sorry about the late reply on this one, I don't really work on angular 1.x stuff anymore. Thanks to the work by @santam85 this is now possible in 0.30.0 🎉

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.