Closed curioussavage closed 9 years ago
@curioussavage Thanks for the pull request! Unfortunately the postedAt date is not accurate right now; it's set to default at the time the study session proposer submits the post, not at the time the session is really scheduled first. I have to fix this problem first before merging in your fix.
(Been meaning to work on it but will take me a couple more weekends.)
Come to think of it I knew that. Lazy commit on my part.
I can update the submission with a date picker, I think there was already one on the admin approval template. I'll update this pull request later today
might be good idea to keep postedAt as the date it was publicly posted and add a scheduledFor date to the post document. Could come in handy for sorting 'posts' by date, since a post could sit for a while before being approved.
All "scheduled" study sessions are automatically approved; there's no wait time for approval. :) But agree that adding a scheduledFor date to the form might be helpful. Let me know if you want to meet at http://codebuddies.org/meteor-hangout sometime tonight or tomorrow morning to IM or screenshare, silent-hangout style.
@lpatmo might be able to chat tonight, or early tomorrow, I am on pacific time.
Realized again I added some stuff not directly related to this issue, feel free to tell me if they wont work for this use case.
Only thing here I think is missing is sorting on that field
W00t, thanks for starting the work on adding it to the form! One thing I'm uncertain of is whether timezone support is built in. People from Ireland/Japan/India/America have sometimes joined in at the same time, and the one thing that confuses everyone is how to convert EST to GMT or vice versa.
Hmm, according to https://github.com/aldeed/meteor-autoform-bs-datepicker the Date objects represents "midnight in the UTC timezone on the morning of the selected date." My worry still is that it won't be clear that the time is EST or PST or GMT, etc.
Oh, also found this: "If you need to use the timezoneId attribute to get the Date in some timezone other than the local client timezone, you must also add moment-timezone:"
... so basically timezone is supported if we add an additional meteor add mrt:moment-timezone
. Will hopefully have some time tomorrow to figure out how exactly it works. https://atmospherejs.com/aldeed/autoform-bs-datetimepicker
So that package is actually being used here.(oops saw you referenced the timezone lib) The browser takes care of showing times converted to the users local timezone.
edit: (as long as the date is created with UTC, which I thought was the default.I will check the format of the dates saved) either way there are methods on the Native Date object to get universal time.
edit: looks like the date object being created now has the local time zone :(
I can understand the worry about timezones, it would be a headache to manage manually.
I was doubting myself so I did some testing on my own app with some help. In my other app I am creating a date object with the native date constructor and storing it in db. On display the users browser is showing it with the correct offset for their timezone.
Thanks for testing! I'll merge your PR and take a look.
Okay, confirmed that the date/time will change depending on the local user's computer clock. Sessions scheduled for "an hour ago" will still say an hour ago, but 9pm will turn into 6pm if the clock is set to San Francisco vs. NYC. Yay!
I merged in your commits after resolving some conflicts, but I think that means I'll have to close this PR, right? Let me know if there's another way I should go about this; mainly, I want to make sure you're credited publicly for helping with open issues, and I'm planning on creating a "leaders" page where I'll have a generated list of the top study session schedulers as well as links to the github accounts of code or design contributors.
Working now on getting the scheduledFor posts listed in order before I deploy to production.
Cool. Have you thought of starting a development branch?
If my branch is merged it should have a message just like above where the commits are. Honestly Ive never submitted a pull request to a repo I don't have write access to. It looks like its showing that you made the changes on your own local branch. It would have a message on this thread saying that you merged my pr
Ah... good suggestion! I should probably do that.
(I do better in IRL, heh; I've just been incredibly lazy with these side projects. Gotta change that.)
Re: pull request... I followed the instructions given ("checkout via command line"); will try again.
Update: worked now!
Yeah, Im not sure how it works when a pull comes from a forked repo. You might have to merge and fix conflicts via github. Not sure though, would Google but the wife wants me to go to bed :)
Haha, got it fixed somehow when retrying! Thanks again for your PR and suggestions. :) Going to bed too.
@curioussavage I created a new branch devel
which you can get to by doing git checkout devel
.
There was a problem that happened with deployment with the new changes (blank screen, though nothing looked wrong on local dev), so I'm going to try to figure out how to push devel
up onto a staging instance to do proper testing.
Maybe you could use meteor deploy and have codebuddies.meteor.com be for staging On Feb 16, 2015 9:51 PM, "lpatmo" notifications@github.com wrote:
@curioussavage https://github.com/curioussavage I created a new branch devel which you can get to by doing git checkout devel.
There was a problem that happened with deployment with the new changes (blank screen, though nothing looked wrong on local dev), so I'm going to try to figure out how to push devel up onto a staging instance to do proper testing.
— Reply to this email directly or view it on GitHub https://github.com/lpatmo/cb/pull/29#issuecomment-74620358.
for issue #9 @lpatmo