-
Notifications
You must be signed in to change notification settings - Fork 718
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for custom end date and start of week to heatmap #66
base: master
Are you sure you want to change the base?
Conversation
How does the start variable work? I am having trouble using it. I find this if I set start myself, with a JS Date object, it limits the number of months generated. It does not do +12 months. |
@viperfx Can you post the code you are using? |
Sure! FYI, I am using it with React, which support was adding in recently.
the start variable would be something like this. 4 months before current date. I was hoping it would do +12 months of the start.
|
I have updated the code so it will default to a duration of one year if only one of |
Sure. So I will need to npm install @ master right? Cause it looks the npm release was not made. |
On the topic of the heatmap, is it possible to add click handlers for each date right now? I can open a new issue for this if you like. |
This is just a pull request that has not been merged yet, I am not one of the official developers of this library. You can download a version containing my proposed changes from https://raw.githubusercontent.com/janten/charts/master/dist/frappe-charts.min.iife.js. |
2c9063b
to
1efedb3
Compare
@janten Glad this satisfies a missing widely-used property. Are there other week systems worth covering? |
@pratu16x7 I don't think so, at least none that I know of. Nevertheless, the change actually supports arbitrary days to start the week since it is interpreted as an offset. Setting |
@janten Cool. We might then need labels for the days as well, for some visual difference between the two settings. Can you rebase the branch? |
What about to change start_monday variable name to first_day_of_week? |
2db5155
to
423dc52
Compare
I see. However, from what I could find, in Bahrain, Egypt, Iraq, Jordan, Kuwait, Oman, Qatar, Saudi Arabia, Syria, the UAE, Yemen and probably a few more Islamic countries, the calendar week starts with Saturday and ends with Friday. I‘m not aware of any countries with calendar weeks starting on Tuesday, Wednesday, Thursday or Friday, though. As there are three rather than two common manifestations of the „first day of the week“, we should definitely use a more neutral variable name than the quasi-boolean All in all, this PR unfortunately has not been finished and merged in all these years, probably to a certain extent because these are two features rolled into one PR, though each of the two features requires quite a bit of discussion. @janten: do you want to pick it up and detangle it into two tickets? Otherwise I would step forward and split it up, while mentioning you as the original author. |
Explanation About What Code Achieves:
This change creates two new attributes,
end
andstart_monday
, to theHeatmap
class.end
works just likestart
andstart_monday
can be set to 1 to have the weeks start on Monday, European style.Screenshots/GIFs:
Steps To Test:
The docs have been added with an example, no further steps necessary.
TODOs:
None.