Skip to content
Permalink
Browse files

Progressbar: Add custom label demo.

  • Loading branch information...
kborchers committed Nov 30, 2012
1 parent f2ee4c5 commit 548a6ce7f927f327e7db261d343caba9b889409d
Showing with 57 additions and 0 deletions.
  1. +57 −0 demos/progressbar/label.html
@@ -0,0 +1,57 @@
<!doctype html>
<html lang="en">
<head>
<meta charset="utf-8">
<title>jQuery UI Progressbar - Default functionality</title>

This comment has been minimized.

Copy link
@scottgonzalez

scottgonzalez Nov 30, 2012

Member

Wrong title.

<link rel="stylesheet" href="../../themes/base/jquery.ui.all.css">
<script src="../../jquery-1.8.3.js"></script>
<script src="../../ui/jquery.ui.core.js"></script>
<script src="../../ui/jquery.ui.widget.js"></script>
<script src="../../ui/jquery.ui.progressbar.js"></script>
<link rel="stylesheet" href="../demos.css">
<style>
.progress-label {

This comment has been minimized.

Copy link
@scottgonzalez

scottgonzalez Nov 30, 2012

Member

Don't indent inside <style>.

float: left;
margin-left: 50%;
margin-top: 5px;
font-weight: bold;
}
</style>
<script>
$(function() {
$( "#progressbar" ).progressbar({
value: false,
change: function( event, ui ) {

This comment has been minimized.

Copy link
@scottgonzalez

scottgonzalez Nov 30, 2012

Member

Don't define unused parameters.

$( ".progress-label" ).text( $( "#progressbar" ).progressbar( "value" ) + "%" );

This comment has been minimized.

Copy link
@scottgonzalez

scottgonzalez Nov 30, 2012

Member

Let's use $( this ).find( ".progress-label" ) or store a reference up front. People will copy this and it'll break with more than 1 progressbar.

This comment has been minimized.

Copy link
@scottgonzalez

scottgonzalez Nov 30, 2012

Member

Actually, we use $( "#progressbar" ) a lot, let's store a reference to that too.

}
});
function progress() {
var val = $( "#progressbar" ).progressbar( "value" );
if ( !val ) {
val = 0;
}
if ( val < 100 ) {
$( "#progressbar" ).progressbar( "value", val + 1 );
setTimeout( function() {

This comment has been minimized.

Copy link
@scottgonzalez

scottgonzalez Nov 30, 2012

Member

setTimeout( progress, 100 );

progress();
}, 100);
} else {

This comment has been minimized.

Copy link
@scottgonzalez

scottgonzalez Nov 30, 2012

Member

This is a recursive loop, so let's put terminal conditions before the main body.

This comment has been minimized.

Copy link
@kborchers

kborchers Dec 4, 2012

Author Member

Maybe I'm misunderstanding but all I am doing before I check the condition is getting the value and making sure it's numeric. Not sure where I could move the conditional.

$( ".progress-label" ).text( "Complete!" );

This comment has been minimized.

Copy link
@scottgonzalez

scottgonzalez Nov 30, 2012

Member

Let's put this inside a complete callback instead. Not only does this keep the labeling inside the callbacks, it avoids an iteration where the label reads "100%" instead of "Complete!". This also makes the terminal condition just stop the iteration.

}
}
setTimeout( progress, 3000 );
});
</script>
</head>
<body>

<div id="progressbar"><div class="progress-label">Loading...</div></div>

<div class="demo-description">
<p>Custom updated label demo.</p>
</div>
</body>
</html>

3 comments on commit 548a6ce

@scottgonzalez

This comment has been minimized.

Copy link
Member

replied Nov 30, 2012

The black on gray text is a little hard to read, especially on top of the indeterminate stripes. Perhaps a slight text shadow in browsers that support it?

Also, I know as soon as this goes live, we're going to have requests for dual labeling. We had this implemented in the original version of progressbar, when it was super bloated. Thoughts on whether we should just do it now or wait until someone sends a PR? See the comments on http://wiki.jqueryui.com/w/page/12138028/Progressbar if you're not sure what I'm talking about with dual labeling.

Overall, I think this is a good demo since it shows how simple it is to customize the display. Thanks :-)

@kborchers

This comment has been minimized.

Copy link
Member Author

replied Dec 4, 2012

Are you saying that we would add dual labeling to the demo or actually add something to Progressbar? I would prefer to not add that to Progressbar. Adding it to the demo I guess would be ok. I'm open to doing that if the majority thinks we should.

@scottgonzalez

This comment has been minimized.

Copy link
Member

replied Dec 4, 2012

Just the demo. I don't have a strong preference either way, I just have a feeling that as soon as this is live, we'll get people requesting it.

Please sign in to comment.
You can’t perform that action at this time.