{"action":"created","issue":{"url":"https://api.github.com/repos/intljusticemission/react-big-calendar/issues/102","repository_url":"https://api.github.com/repos/intljusticemission/react-big-calendar","labels_url":"https://api.github.com/repos/intljusticemission/react-big-calendar/issues/102/labels{/name}","comments_url":"https://api.github.com/repos/intljusticemission/react-big-calendar/issues/102/comments","events_url":"https://api.github.com/repos/intljusticemission/react-big-calendar/issues/102/events","html_url":"https://github.com/intljusticemission/react-big-calendar/pull/102","id":157348043,"number":102,"title":"Implement unified TimeGutter in Week and Day view using refactored code","user":{"login":"cellog","id":98250,"avatar_url":"https://avatars.githubusercontent.com/u/98250?v=3","gravatar_id":"","url":"https://api.github.com/users/cellog","html_url":"https://github.com/cellog","followers_url":"https://api.github.com/users/cellog/followers","following_url":"https://api.github.com/users/cellog/following{/other_user}","gists_url":"https://api.github.com/users/cellog/gists{/gist_id}","starred_url":"https://api.github.com/users/cellog/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/cellog/subscriptions","organizations_url":"https://api.github.com/users/cellog/orgs","repos_url":"https://api.github.com/users/cellog/repos","events_url":"https://api.github.com/users/cellog/events{/privacy}","received_events_url":"https://api.github.com/users/cellog/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":13,"created_at":"2016-05-28T17:10:12Z","updated_at":"2016-06-04T21:07:22Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/intljusticemission/react-big-calendar/pulls/102","html_url":"https://github.com/intljusticemission/react-big-calendar/pull/102","diff_url":"https://github.com/intljusticemission/react-big-calendar/pull/102.diff","patch_url":"https://github.com/intljusticemission/react-big-calendar/pull/102.patch"},"body":"changes:\r\n\r\n1. re-factored TimeSlice/TimeSliceGroup in TimeGutter removes some loop code in favor of mapping, makes selection easier to implement\r\n2. re-factored DaySlot uses same code as TimeGutter to generate html\r\n3. re-factored selection code reduces code size substantially, separates concerns (selection is not handled within components, but with higher-order components Selection and Selectable)\r\n4. otherwise, it's the same!  Visually functioning the same way, external API identical\r\n\r\nPotential API to expose:\r\n\r\n1. because the selection is now configurable, it is possible to expose the function for creating the selection div\r\n2. time slices, can make smaller slices (up to 5 per group) default is 2\r\n3. rather than using new Date() for now, we should pass this as a prop, with default value of new Date().  Makes the component slightly more pure.\r\n\r\npotential bugs to fix (pre-existing behavior):\r\n\r\n1. selection of the all-day slots doesn't do anything (and never did).  Perhaps we need to expose onSelectAllDay?\r\n2. react/react-dom can be upgraded to 15.1.0.  I left that for you to decide, I don't use anything here that breaks in earlier versions to my knowledge\r\n\r\nP.S. had a nice vacation, hope your week was good :)"},"comment":{"url":"https://api.github.com/repos/intljusticemission/react-big-calendar/issues/comments/223778386","html_url":"https://github.com/intljusticemission/react-big-calendar/pull/102#issuecomment-223778386","issue_url":"https://api.github.com/repos/intljusticemission/react-big-calendar/issues/102","id":223778386,"user":{"login":"jquense","id":339286,"avatar_url":"https://avatars.githubusercontent.com/u/339286?v=3","gravatar_id":"","url":"https://api.github.com/users/jquense","html_url":"https://github.com/jquense","followers_url":"https://api.github.com/users/jquense/followers","following_url":"https://api.github.com/users/jquense/following{/other_user}","gists_url":"https://api.github.com/users/jquense/gists{/gist_id}","starred_url":"https://api.github.com/users/jquense/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jquense/subscriptions","organizations_url":"https://api.github.com/users/jquense/orgs","repos_url":"https://api.github.com/users/jquense/repos","events_url":"https://api.github.com/users/jquense/events{/privacy}","received_events_url":"https://api.github.com/users/jquense/received_events","type":"User","site_admin":false},"created_at":"2016-06-04T21:07:22Z","updated_at":"2016-06-04T21:07:22Z","body":"one more note:\r\n\r\n> So for this to be productive, I need a clearer sense of requirements, or of things that cannot change. \r\n\r\nI agree with you on this, but it should be the other way around, there needs to be a strong reason _to_ change things, before we change things, because new code always has a cost. I appreciate that you see that there _are_ reasons to change them and that trying to describe code limitations is hard. \r\n\r\nFrom my side its also really difficult to see why code doesn't work for you when its been removed completely. I also hear you when you say the need code is functionally similar (or the same) as the old code which is great, however my impulse then in that case is \"If its not much different then lets not change it at all, its not worth have new code\". Also at a high level I hear what you are trying to do, but I am not sure what that means implementation wise. Are you planning on using all the internal components apart from the Calendar itself? That may be fine, but we'd have to weigh whether supporting that costs something for the main purpose of providing a mostly batteries included Scheduler component, or the cost of now vastly increasing the size of the public API. \r\n\r\nSo we need to get more specific: \r\n\r\n- What does the new code allow that the old did not, and conversely Why not was tweaking the old code not tenable (specifics). \r\n\r\nI agree that it's tough to explain that stuff, but there is also no way for me to jump in and try and see what we are seeing, because all the old Selection code is gone."}}
{"actor":{"display_login":"jquense"}}