Skip to content
This repository has been archived by the owner on Jul 24, 2018. It is now read-only.

Feature/alert new post #6

Merged
merged 5 commits into from Apr 16, 2014

Conversation

dombioinfo
Copy link
Contributor

Add blink effect when new message is received

@pypingou
Copy link
Member

Maybe we should remove all the commented console.log directly, I'm not sure there is much interest in keeping them in.

@@ -182,5 +182,39 @@ function setup_websocket_listener() {
// the websocket connection, but we have to go query again to
// get the fedmsg.meta information.
update_fedmsg(id_lookup[category], category, deploy);

// Add blink effect on page's title to alert user of new post
// Note: this code is adapted from canop's blog (canop.org)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be worth putting the whole url.

Is there a license on the code there?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In fact, canop re-writes in his "words" what it is explained on page : https://developer.mozilla.org/en-US/docs/Web/Guide/User_experience/Using_the_Page_Visibility_API?redirectlocale=en-US&redirectslug=DOM%2FUsing_the_Page_Visibility_API which license is "Creative Commons Attribution-ShareAlike".

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's actually public license:

Code samples added on or after August 20, 2010 are in the public domain. No licensing notice is necessary, but if you need one, you can use: "Any copyright is dedicated to the Public Domain. http://creativecommons.org/publicdomain/zero/1.0/".

So from a license point of view, it's ok to merge it, just add the full link please.

} else {
blinkTimer = setInterval(function() {
var title = document.title;
document.title = (title === originalTitle) ? "New " + id_lookup[category] : originalTitle;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about moving the test up a little so that we do no update the document.title unless there is a change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This effect is applied at each received message (from all categories). Perhaps we can apply this effect only when we receive messages which its categroy is 'planet'.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's worth keeping it for all pages

@pypingou
Copy link
Member

I have this running for a while now and it works good, so let's merge it :)

pypingou added a commit that referenced this pull request Apr 16, 2014
@pypingou pypingou merged commit cf0001f into fedora-infra:develop Apr 16, 2014
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

2 participants