search

BradleyCai / ucr-schedule-visualizer

Generates an hour by hour view and shows the locations of all the classes from a raw class list from GROWL.
https://waa.ai/ucrsv
MIT License
5 stars 2 forks source link

Added Google Calendar integration #86

Open blamejoel opened 8 years ago

blamejoel commented 8 years ago

Demo available here. I've left my api key in the repo for convenience and have already authorized it to work from bradleycai.github.io.

It's not a perfect implementation in my eyes, but it works.

emmiegit commented 8 years ago

Thanks for submitting the pull request! We really appreciate your work.

We'll spend a couple days reviewing the changes, and in the mean time we'll be posting comments in both this issue and on the diff. In the mean time, you can join our Discord server so that, if you wish, we can discuss the issue in real time.

emmiegit commented 8 years ago

1475040594

If "visualize again" is selected, the new button duplicates itself. This bug has happened before, we'll look to see what we did to fix it.

emmiegit commented 8 years ago

We already have a function that calculates the starting and ending points for each quarter. See QuarterDates.js, starting from line 10. Bit of a shame, since it's a cool utility.

blamejoel commented 8 years ago

Yea, I'm not calculating them, I'm literally just scraping them from the UCR Academic Calendar page and spitting them out.

I did notice the bug with the button, and that it clears with a refresh. I figured there was some logic elsewhere used for the other buttons for destroying them, but I didn't look into it. For the button, I just uncommented what was already there for it (I think in js/Main.js), and moved it to where to the other buttons were being created.

Joel Gomez Undergraduate, Computer Science Bourns College of Engineering University of California, Riverside

On Tue, Sep 27, 2016 at 10:35 PM, Ammon Smith notifications@github.com wrote:

We already have a function that calculates the starting and ending points for each quarter. See QuarterDates.js, starting from line 10 https://github.com/BradleyCai/ucr-schedule-visualizer/blob/gh-pages/js/QuarterDates.js#L10. Bit of a shame, since it's a cool utility.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/BradleyCai/ucr-schedule-visualizer/pull/86#issuecomment-250076131, or mute the thread https://github.com/notifications/unsubscribe-auth/AOaY6TlDEdfOgxDBI0wSIQh8w2tfOXnrks5qufy-gaJpZM4KIaZG .

emmiegit commented 8 years ago

Merging the changes from 67f1b286 into your PR should fix the merge conflict.

emmiegit commented 8 years ago

Yea, I'm not calculating them, I'm literally just scraping them from the UCR Academic Calendar page and spitting them out.

Unfortunately, that's the only way to get 100% correct dates, but it's a bit slow to do so. We looked at past years and QuarterDates should work to be reasonably accurate at predicting future quarter dates. Mostly it's the benefit of not having to do a fetch on a remote web resource.

blamejoel commented 8 years ago

No worries, use what you like. It's just a few minor changes to use QuarterDates, just provide the date in the ISO format needed for the calendar event.

Joel Gomez Undergraduate, Computer Science Bourns College of Engineering University of California, Riverside

On Tue, Sep 27, 2016 at 10:42 PM, Ammon Smith notifications@github.com wrote:

Yea, I'm not calculating them, I'm literally just scraping them from the UCR Academic Calendar page and spitting them out.

Unfortunately, that's the only way to get 100% correct dates, but it's a bit slow to do so. We looked at past years and QuarterDates should work to be reasonably accurate at predicting future quarter dates. Mostly it's the benefit of not having to do a fetch on a remote web resource.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/BradleyCai/ucr-schedule-visualizer/pull/86#issuecomment-250076950, or mute the thread https://github.com/notifications/unsubscribe-auth/AOaY6X89zqdAMqQRiqyeCOOrH3MvTQ3wks5quf5BgaJpZM4KIaZG .

emmiegit commented 8 years ago

A few development notes:

BradleyCai commented 8 years ago

Took a look at the changes and noticed a couple small things that could be polished.

First the "Sign in with google" modal wording could be changed a bit.

image

Instead of "Oops" it could be "Sign in". The description could be a little more friendly. Something like "In order to add the schedule to your calendar please sign in." Just something short, simple, and fast to read. Also there's no need for two buttons that dismiss the box. The text is also bigger than the rest of the website's font.

Secondly perhaps it'd be nicer to have the schedule export to it's own separate calendar

image

That way it doesn't tangle with the user's other calendars. It should also have a specific name like the fall 2015 calendar does.

Third, it'd be preferable if the "Visualize Again" button was on the far right.

image

The "cancel" and "no" button in most dialog boxes are seen on the far right so I think "Visualize Again" should be too.

In the next couple of days I'll review the code in detail and once that's done we'll pull the request down. Nice work on the issue and thanks for contributing!

blamejoel commented 8 years ago

TBH, once I got it all working I thought the modal is kinda pointless. Google creates a new window to authorize the app regardless, so it seems like one more unnecessary pop up. The main reason I slapped it in to begin with was to let people know WHY Google was about to ask them to give permission. I would say lose the modal all together... Just know that if the user hasn't already authorized the app, they're going to get the pop up window asking them to do so.

Also, I think there should probably be an alert text drawn on the page temporarily to let users know that the events were added successfully or if there were any errors. Maybe THAT could be a modal, complete with links to each "event" that was added.

I can look into creating a new calendar for the events. The only thing that I think would need to be different would be to create the "UCR [quarter] [year] Classes" calendar on the user's account, then we'd need to read all their calendars to get the correct calendar ID for the one that the courses should be added to and just plug in that ID into the function that's already there to create the events.

BradleyCai commented 8 years ago

@jgome043 Yeah, good point. Since the oauth window from google pops up and asks the user for their calendar permissions there's no need for a modal along with it. A modal popping up after the calendar is successfully created sounds good though.

Ammon and I were thinking about implementing a larger more scalable notification system along with rewriting a lot of the code to be more modular (see issue #85), but for now you can just use a modal notification until we get to it.

Also the schedule in a separate calendar is going to be important since some people move around classes and change sections and they want to be able to switch around schedules. If each schedule pushed to google calendar wasn't in it's own calendar, that'd be a nightmare to clean up and re-enter.