Opened 12 years ago

Closed 12 years ago

Last modified 9 years ago

#2803 closed defect (fixed)

ProgressBar: avoid sizing during creation

Reported by: Adam Peller Owned by: Adam Peller
Priority: high Milestone: 0.9
Component: Dijit Version: 0.4.2
Keywords: Cc: bill
Blocked By: Blocking:

Description

In ProgressBar?, from Bill's e-mail:

this.frontLabel.style.width = dojo.getComputedStyle(this.domNode).width;

We don't want to do javascript sizing, especially during creation, and
especially on a node that's likely to be originally hidden.   Can't you
just do:

  this.frontLabel.style.width = "100%";

Assuming that frontLabel has no border or padding (or margin?) I think
this should work fine on all browsers.

Change History (16)

comment:1 Changed 12 years ago by simonjb

The frontLabel is actually a child of the progress bar div (rather than the container) so if we set the width=100% it fills the width of the bar rather than the container.

I know there have been some thoughts given to removing the 2 label divs and using 1 instead. If we did that then the label div could sit on top of the bar with width=100% and we wouldn't need the sizing in JavaScript.

comment:2 Changed 12 years ago by bill

Component: WidgetsDijit

comment:3 Changed 12 years ago by bill

OK, I've looked at this code some more and talked w/Simon on IRC.

First of all, the attach points "internalProgressTileLayer" and "internalProgressColorLayer" are unused and should be removed, to avoid confusion.

Second, I think the sizing of the bar can be changed to just use %:

var pixels = percent * parseInt(dojo.getComputedStyle(this.domNode)[this._dimension]);
this.internalProgress.style[this._dimension] = pixels + 'px';

can be changed to

this.internalProgress.style[this._dimension] = percent + "%";

Finally, the dual-tone text is tricky because the text contained in the bar (the "fullLabel") has to be longer than the bar itself (ex: a 300px progress bar widget at 25% will have 75px bar, but the bar contains text at 300px). But it's worth trying to set the bar width to 100/percent, rather than to a pixel value. In other words:

this.fullLabel.style[this._dimension] = 100/percent + "%";

Worth a shot anyway.

comment:4 Changed 12 years ago by bill

Resolution: fixed
Status: newclosed

(In [8861]) * fixes high-contrast mode (fixes #3134)

  • now using dijit_a11y to implement high-contrast, removed special bordered div
  • made bar visible without tundra (just dijit.css) (background-color:#aaa)
  • bar sizing is now set by percent rather than pixels (fixes #2803)
  • fixed label positioning on vertical progress bar (fixes #3107)
  • removed the annotate property (fixes #3204)

Patch from Simon Bates (CLA on file)

comment:5 Changed 12 years ago by bill

Resolution: fixed
Status: closedreopened

Turns out there's still some sizing stuff in there, including in the postCreate() call. Not the end of the world but we should remove it if possible. (Related to #3231)

comment:6 Changed 12 years ago by simonjb

Milestone: 0.9beta0.9

comment:7 Changed 12 years ago by simonjb

Milestone: 0.91.0

comment:8 Changed 12 years ago by Adam Peller

Simon, I'm cool with putting this off to 1.0 for tuning purposes, but I see a problem in the themeTester with ProgressBar? that I think is related to the addition of some code setting the bottom positioning. Can you take a look?

comment:9 Changed 12 years ago by bill

The other thing is that this probably breaks having a progress bar in a Dialog which is bad since that's the typical use case. (Not sure why this bug is assigned to Simon.)

comment:10 Changed 12 years ago by Adam Peller

Cc: Adam Peller removed
Owner: changed from simonjb to Adam Peller
Status: reopenednew

comment:11 Changed 12 years ago by Adam Peller

Resolution: fixed
Status: newclosed

(In [9815]) Remove sizing during postCreate and clean up some redundant CSS and JS. Fixes #2803

comment:12 Changed 12 years ago by Adam Peller

(In [9816]) Handle BiDi? special case in CSS. Refs #2803

comment:13 Changed 12 years ago by Adam Peller

Milestone: 1.00.9

comment:14 Changed 12 years ago by Adam Peller

(In [9818]) Avoid div by zero. Refs #2803

comment:15 Changed 12 years ago by Adam Peller

(In [9821]) Remove computed style setting for bottom. Refs #2803

comment:16 Changed 9 years ago by bill

(In [21614]) Remove some FF workaround code that's apparently no longer needed, although hard to confirm since there is no record of why it was needed to begin with. Refs #2803, #10816.

Note: See TracTickets for help on using tickets.