#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 15 years ago by
comment:2 Changed 15 years ago by
Component: | Widgets → Dijit |
---|
comment:3 Changed 15 years ago by
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 15 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
(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 15 years ago by
Resolution: | fixed |
---|---|
Status: | closed → reopened |
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 15 years ago by
Milestone: | 0.9beta → 0.9 |
---|
comment:7 Changed 15 years ago by
Milestone: | 0.9 → 1.0 |
---|
comment:8 Changed 15 years ago by
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 15 years ago by
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 15 years ago by
Cc: | Adam Peller removed |
---|---|
Owner: | changed from simonjb to Adam Peller |
Status: | reopened → new |
comment:11 Changed 15 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
comment:13 Changed 15 years ago by
Milestone: | 1.0 → 0.9 |
---|
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.