-
Notifications
You must be signed in to change notification settings - Fork 49
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
new contest notifier [Chrome] #26
new contest notifier [Chrome] #26
Conversation
642428d
to
4df19a2
Compare
@@ -0,0 +1,22 @@ | |||
var req = new XMLHttpRequest(); | |||
function getURLs(){ | |||
req.open("GET","http://contesttrackerapi.herokuapp.com/contestURLs",true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we really need an extra route?
Can't we get the same info from the index route?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is just to optimize the network usage as the bg.js will fetch these URLs every 1 minute. Fetching the whole json doesn't make sense, it will waste network data.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to update the badge every minute?
Our aim is to notify the user that a new contest has been announced.Generally contests are announced at least a couple of days before the contest starts.So alerting the user as soon as it is announced seems like overkill to me.
I don't think it makes a difference if a user is alerted about a new contest only after a few minutes after it has been announced
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can change the update interval to 10-20 mins.
Yeah the aim is to notify the user about new contest and that's what exactly this will do. I did in a way how general notification feature of any product works. It checks if a new contest has been added or not after a regular interval.
If I add the code of setBadgeText in popup,js, It will be useless to display number on top of the extension icon, as the user will be able see new contest marked with new tag in the list.
Why add a separate background script for setting the BadgeText? Why not do it from inside the popup.js during fetchdata? |
bg.js will be running all the time in the background after the installation of the extension which is required to check if a new contest has been added or not. popup,js only gets executed when the extension icon is clicked. |
Oh yeah.That makes sense. |
chrome.browserAction.setBadgeBackgroundColor({ color: "#4c9bff"}); | ||
} | ||
}; | ||
setTimeout(getURLs,60000); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove the setTimeout and instead use setInterval outside getURLs and also change the interval to 30mins
Please make the following changes:
|
if __name__ == '__main__': | ||
port = int(os.environ.get("PORT", 5000)) | ||
app.run(host='0.0.0.0', port=port,debug=True) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reset all changes to this file since nothing is being done here.
when ever there is a new contest in the list a small number will appear on top of the icon of the GC extension representing the number of new contest added
f1aed51
to
51ce708
Compare
LGTM! Thank you for your contribution. |
New Contest Notification Feature added.
Shows a number on top of icon of the GC extension whenever a new contest appears in the list.
Also a tag is displayed on top of the contest marking it as a new contest.