Skip to content
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

Added desktop notifications to Chrome extension #7

Merged
merged 9 commits into from
Mar 9, 2015

Conversation

cdrini
Copy link
Contributor

@cdrini cdrini commented Mar 8, 2015

Added desktop notifications to the chrome extension when results are received (see #3 ). Here are some samples:

Notification from ProblemsList:
image
Clicking "View results" opens a new tab to the submission ("submission.jsp?submissionPK=...")
Clicking the notification brings focus to the marmoset tab if still open, else opens a new one.

Notification from SubmissionsList:
image
Clicking the notification brings focus to the marmoset tab if still open, else opens a new one.

@lishid
Copy link
Owner

lishid commented Mar 8, 2015

Looks great!

I'm curious whether the notification shows up if you're already on the page, or does Chrome hide/prevent the notification if you're already on page.

Another thing I'm curious about is that it "brings focus to the marmoset tab if still open".
Does the notification work if you close the tab? I would think that once the tab closes, the async request is gone and therefore cannot trigger the notification to show up. Am I wrong?

Other than those, it's looking pretty nice. I'll pull it in a bit and give it a test run!

@cdrini
Copy link
Contributor Author

cdrini commented Mar 8, 2015

Glad you like it!
Currently the notification will show up even if you're already on the page, but if you'd like, I don't think it would take me too long (5 lines?) to add that feature.

The case is if marmoset shows the notification, and the user closes marmoset while the notification is still being displayed. Relatively extreme edge case, to be honest, though.

@lishid
Copy link
Owner

lishid commented Mar 8, 2015

I think it would be useful to prevent the notification if you're already looking at the page.

Another thing I was curious is whether Chrome lets you disable notifications somehow. If such is not the case, some might find it annoying for notifications to popup while they're working on other things, and we might want a settings page for disabling it.

@cdrini
Copy link
Contributor Author

cdrini commented Mar 9, 2015

Ok, the notification will no longer show if you have the page open.

And chrome does have a setting to disable notifications:
image

I made sure the app works correctly if they are disabled.

(I did create an options page before finding this, though. In case that's ever useful, I have it saved as a branch on my fork: https://github.com/cdrini/MarmoUI/tree/optionsPage )

@cdrini
Copy link
Contributor Author

cdrini commented Mar 9, 2015

P.S.
A quick way to test notifications is to type the following in the browser console when marmoset is open:

setTimeout(function() {window.postMessage({type:"resultsNotification", notification:{}},"*")}, 1000)

It will just make a junk notification, though. Be sure to leave the marmoset tab/window before the timeout ends!

lishid added a commit that referenced this pull request Mar 9, 2015
Added desktop notifications to Chrome extension
@lishid lishid merged commit 754795d into lishid:master Mar 9, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants