-
Notifications
You must be signed in to change notification settings - Fork 47
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
consider removing timezone functionality #58
Comments
@jamesdixon I agree with you. It will be better to configure moment timezone outside. |
I think this makes sense, as long as timeZone still can be provided as an argument, the ability to change timezone from within the calendar can be removed. |
@toreriklinnerud thanks for the feedback. When you mention providing timezone as an argument, what purpose would that serve? My proposal is that the calendar becomes completely timezone-agnostic -- it only cares about displaying occurrences and leaves the manipulation of those occurrences outside of the component. Sorry if I'm missing something! |
this is next on my list |
We've started working on a branch that does this. We're running it in prod at the moment and it's working ok. I'd like to make some helper functions that help people migrate over with a bit more ease. We're using these in our app atm so bringing them across shouldn't be too difficult. Because this is not backwards compatible we'll release it as a major release. |
While the timezone functionality is a neat feature, I'm assuming this is probably not used terribly often. I could be wrong, but would love to hear feedback from others.
That said, my proposal would be to remove the timezone functionality. It would allow the component to be leaner (
moment-timezone
is almost 3mb) and simpler. This would also allow the removal of the dropdown dependency in addition to liquid fire (#56 ). For those that need the timezone functionality, this could easily be handled outside of the calendar, which I believe it ultimately should be.I realize this would be a breaking change, but I believe it would be worthwhile.
Note: I'm willing to help with these things, so don't think I'm just posting a ton of feature requests 😄
The text was updated successfully, but these errors were encountered: