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 rotating tips. 5 tips that cycle every 60 seconds with fadeout/fadein #390

Merged
merged 7 commits into from Feb 27, 2012
Merged

- Added rotating tips. 5 tips that cycle every 60 seconds with fadeout/fadein #390

merged 7 commits into from Feb 27, 2012

Conversation

PureKrome
Copy link
Contributor

5 Tips :-

  • Type @ then press TAB to auto-complete nicknames
  • Type /help to see the list of commands
  • Type /rooms to list all available rooms
  • Type : then press TAB to auto-complete emoji icons
  • You can create your own private rooms. Type /help for more info

Fade Out / Fade In = 2 secs each.

Rotate is set for every 60 seconds.

** Apologies for the 2nd commit. I wanted to get my style, right.

});

$('#message-instruction').fadeIn(2000, recursiveTimeout());
}, 60 * 1000);
Copy link
Member

Choose a reason for hiding this comment

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

Make this a constant.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

do you mean, the time in milliseconds?

Copy link
Member

Choose a reason for hiding this comment

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

something readable.

- Refactored the setTimeout time to a constant.
@PureKrome
Copy link
Contributor Author

Updated.

var cycleTimeInMilliseconds = 60 * 1000; // 1 minute.
var currentTipIndex = 0;

function recursiveTimeout() {
Copy link
Member

Choose a reason for hiding this comment

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

Change name to cycleMessages

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

var cycleTimeInMilliseconds = 60 * 1000; // 1 minute.
var cycleMessageIndex = 0;

function recursiveTimeout() {
Copy link
Member

Choose a reason for hiding this comment

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

Change the name of recursiveTimeout to cycleMessages...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated.
(sorry about the slow update -> weekends, kids, no life, QQ....)

- Renamed the messages array.
@davidfowl
Copy link
Member

Why is this code in index.htm?

@PureKrome
Copy link
Contributor Author

3 reasons.

  1. The message div is located in this file.
  2. it's tiny.
  3. i didn't know of any other js file where it would be appropriate to place it, in. Having it in a seperate js file is preferable because of caching.

I would prefer to have it in a js file. Any preference? chat.ui.js?

@davidfowl
Copy link
Member

Yes move it to the ui file :).

…Chat.ui.js

- CycleMessages are now started when the document has finished loading.
@PureKrome
Copy link
Contributor Author

Ok. done.

few points to note:

  1. I'm kick starting the function logic by calling the jquery ready logic. I'm not 100% sure if this is the best place to do it, so I'm happy to move this starting call to a more appropriate place (i'm just not 100% familiar with the js jabbr code).
// Start cycling the messages once the document has finished loading.
$(document).ready(cycleMessages());
  1. Even though i've added a variable to define the cycle time in milliseconds, I'm not sure if that's an overkill. So i'm also happy to replace this..

var cycleTimeInMilliseconds = 60 * 1000; // 1 minute. $('#message-instruction').fadeIn(2000, cycleMessages()); }, cycleTimeInMilliseconds);

with this

$('#message-instruction').fadeIn(2000, cycleMessages()); }, 60000);

};

// Start cycling the messages once the document has finished loading.
$(document).ready(cycleMessages());
Copy link
Member

Choose a reason for hiding this comment

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

Yea this is all wrong and doesnt follow the pattern we use today. Kick it off in ui.initialize.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated. (added to the bottom of the ui.init. I didn't know that function existed. Awesome!)

…g to

  ui.initialize function. (Cheers DFowler)
- BugFix: a function calla was getting invoked immediately. Refactored to be
  a function reference instead. (Cheers redsquare)
davidfowl added a commit that referenced this pull request Feb 27, 2012
- Added rotating tips. 5 tips that cycle every 60 seconds with fadeout/fadein
@davidfowl davidfowl merged commit f90bc2f into JabbR:master Feb 27, 2012
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.

None yet

2 participants