Skip to content
This repository has been archived by the owner on Dec 5, 2022. It is now read-only.

Set Reminder frontend - Fixes #158 #1232

Merged
merged 1 commit into from
Apr 29, 2015
Merged

Set Reminder frontend - Fixes #158 #1232

merged 1 commit into from
Apr 29, 2015

Conversation

ebidel
Copy link
Contributor

@ebidel ebidel commented Apr 29, 2015

@ebidel
Copy link
Contributor Author

ebidel commented Apr 29, 2015

@crhym3 am I doing anything with iostart or is that @jeffposnick ?

@ebidel ebidel changed the title Fixes #158 Set Reminder frontend - Fixes #158 Apr 29, 2015
@x1ddos
Copy link
Contributor

x1ddos commented Apr 29, 2015

Monica said:

"Set a reminder" should add a separate reminder for the beginning of I/O, which is actually at 9am with a pre-Keynote show. When a user sets a reminder for I/O tho, ideally, they will see a notification the day before letting them know I/O is starting soon.

That's why I created iostart. AFAIU #158 should be done in a way similar to what "I want to receive notifications" checkbox in the signin settings does, calling PUT /api/v1/user/notify.

@ebidel
Copy link
Contributor Author

ebidel commented Apr 29, 2015

This goes through the signin+enable notification flow. I was thinking the backend could do the bit where it sends a notification the day before the keynote session starts.

@x1ddos
Copy link
Contributor

x1ddos commented Apr 29, 2015

Ah, yeah that could work! But I thought you also needed a data bit iostart to reflect this in the UI?

What if I don't want to be notified 1 day before but only when keynote starts. Is that a valid user case?

@jeffposnick
Copy link
Contributor

The code looks good. It's kind of a shame that a bunch of the code from schedule.html needs to be copied over, but at the same time, there are a number of strings and variable bindings that would be messy to parameterize if the code were refactored into a helper function.

Re: iostart vs. __keynote__, I'm not sure either, and will discuss that bit with you all off-thread.

@ebidel
Copy link
Contributor Author

ebidel commented Apr 29, 2015

@jeffposnick and I talked. We're just going to hook this up to the __keynote__ for now and allow uses to keep pressing the "set reminder" button. It will also help users if they open another browser and are not signed in. It will take them through that flow.

ebidel added a commit that referenced this pull request Apr 29, 2015
Set Reminder frontend - Fixes #158
@ebidel ebidel merged commit 7803575 into master Apr 29, 2015
@ebidel ebidel deleted the 158 branch April 29, 2015 16:24
@monicabagagem
Copy link

If this will be triggered by the keynote, then this is redundant as we're already sending notifications for the keynote to everyone who is opted-in to receive notifications. Can't we just time base this notification for May 27?

also, just a reminder that the keynote notification should happen at 9:20am vs 8:50am (as originally planned).

@monicabagagem
Copy link

(unless something has changed and we're no longer adding keynote by default to my schedule?)

@ebidel
Copy link
Contributor Author

ebidel commented Apr 30, 2015

We discussed (#158 (comment)) leaving this as an additional path for opting into notifications. Users won't get a notification for anything unless they through that flow.

@monicabagagem
Copy link

right - we discussed having "Set a reminder" for the day before since we already have a reminder by default for the keynote (and as an extra reminder to turn on notifications).

But per this thread, it seems you want "Set a reminder" to be associated with the Keynote instead? (which i don't agree with, since it's redundant for the users!)

@ebidel
Copy link
Contributor Author

ebidel commented Apr 30, 2015

We still need users to opt-in to notifications so it's not entirely
redundant. Sign in prompts to enable the browser permission now, but it's
easy for users to dismiss that.

Eng chatted about this this morning. We all agreed that it's better to err
on the side of caution with notification stuff. Jeff made the point that
the last thing we want is to send dozens of notifications from now until
I/O, be too spammy, have users end up turning the feature off.

Keeping it to the keynote is the easiest solution and is already
implemented. We can still show this particular notification 24hrs before,
10min, or whatever we would like.

On Wed, Apr 29, 2015 at 9:48 PM Monica Bagagem notifications@github.com
wrote:

right - we discussed having "Set a reminder" for the day before since we
already have a reminder by default for the keynote (and as an extra
reminder to turn on notifications).

But per this thread it seems, you want "Set a reminder" to be associated
with the Keynote instead? (which i don't agree with, since it's redundant
for the users!)


Reply to this email directly or view it on GitHub
#1232 (comment)
.

@monicabagagem
Copy link

I would rather have these as 2 separate reminders but if everyone agrees that this makes sense to associate with the keynote one, then we should send it perhaps 30 min before at 9am (to give people some time)?

What does it say currently:
"The Google I/O keynote starts in 30 min" ---> prefered
OR
"The Google I/O keynote starts at 9AM PDT"

@ebidel
Copy link
Contributor Author

ebidel commented Apr 30, 2015

@jeffposnick
Copy link
Contributor

I just created #1248 to track notifications for sessions starting (including the keynote), tied to Phase 3.

@ebidel
Copy link
Contributor Author

ebidel commented Apr 30, 2015

Thanks Jeff

On Thu, Apr 30, 2015 at 10:26 AM Jeffrey Posnick notifications@github.com
wrote:

I just created #1248
#1248 to track
notifications for sessions starting (including the keynote), tied to Phase
3.


Reply to this email directly or view it on GitHub
#1232 (comment)
.

@x1ddos
Copy link
Contributor

x1ddos commented Apr 30, 2015

don't forget the videos!
On 30 Apr 2015 7:39 pm, "Eric Bidelman" notifications@github.com wrote:

Thanks Jeff

On Thu, Apr 30, 2015 at 10:26 AM Jeffrey Posnick <notifications@github.com

wrote:

I just created #1248
#1248 to track
notifications for sessions starting (including the keynote), tied to
Phase
3.


Reply to this email directly or view it on GitHub
<
https://github.com/GoogleChrome/ioweb2015/pull/1232#issuecomment-97888336>
.


Reply to this email directly or view it on GitHub
#1232 (comment)
.

@jeffposnick
Copy link
Contributor

I won't forget the videos! But I mean, that's what, Phase 4? Who can think that far ahead? 😄

@monicabagagem
Copy link

monicabagagem commented Apr 30, 2015 via email

@jeffposnick
Copy link
Contributor

Ah, okay. Let me track that in a separate issue then.

@jeffposnick
Copy link
Contributor

Actually, are we showing notifications for videos that aren't tied to sessions? I thought we scrapped that because there's no way to opt-in for a random DevByte notification, and we were only going to notify you when a session in My Schedule had its video published.

@monicabagagem
Copy link

Ah yes, for notifications this only impacts P4!
I was referring to the Video page, which needs to come out in P2.
We're all good!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants