Opened 16 years ago
Closed 15 years ago
#894 closed enhancement (invalid)
[patch][cla]Accordion enhancement to allow setting of panel size (bug portion of this ticket already fixed)
Reported by: | Owned by: | Adam Peller | |
---|---|---|---|
Priority: | high | Milestone: | |
Component: | Dijit | Version: | 0.2 |
Keywords: | Cc: | [email protected]… | |
Blocked By: | Blocking: |
Description
After fixing the problem with dojo.style.getUnitValue(), the AccordionPane? widget was still having problems in Safari when it was created in script within a div that had display:none. It turned out that the problem was in the setSizes() method which was doing math to calculate how large the containerNode should be. Unfortunately, it was receiving NaN as a value and then doing math with it, which resulted in a value equal to NaN as well. Needless to say, ll the containerNodes wound up with a height of 0px.
Here is a simple patch which just checks that the value is a value before allowing it to be used in height calculations.
I have also attached a copy of a subclass of AccordionContainer? that I am now having to use which adds a method which can be called AFTER the container is displayed so that the sizes can be recomputed. It also lets you specify how big each pane will be and sets the size of the parent node accordingly. I'd love to see it integrated into dojo, but will continue to use my subclass if not.
Attachments (2)
Change History (16)
Changed 16 years ago by
Changed 16 years ago by
Attachment: | TacosAccordionContainer.js added |
---|
comment:1 Changed 16 years ago by
Owner: | changed from anonymous to bill |
---|---|
Status: | new → assigned |
comment:2 Changed 16 years ago by
comment:3 Changed 16 years ago by
Milestone: | → 0.4 |
---|
Do we have a CLA on file? Is this still an issue?
comment:4 Changed 16 years ago by
Cc: | [email protected]… added |
---|
Hey Sam...Adding you in...I think they want your CLA :)
comment:5 Changed 16 years ago by
My CLA was filed the same day that these bugs were added. I even discussed it with Alex. I scanned and emailed it to him and I know he received it.
comment:6 Changed 16 years ago by
Confirmed that CCLA was signed and received by Alex Russell on June 03 of this year I just forwarded his email confirming receipt back to him.
comment:7 Changed 16 years ago by
Owner: | changed from bill to Adam Peller |
---|---|
Status: | assigned → new |
from [email protected] (CCLA on file) I believe the patch to AccordionPane?.js for problem #894 is unnecessary and does not fix the real problem. I tracked the NaN problem down to src/html/layout.js line 277: if(typeof args.width != undefined){ should be if(typeof args.width != "undefined"){ Note the quotes. Same problem with lines 281, 301, 306
comment:8 Changed 16 years ago by
Summary: | AccordionPane can't handle being created with display:none in Safari → [patch][cla]AccordionPane can't handle being created with display:none in Safari |
---|
I believe the patch to AccordionPane?.js for problem #894 is unnecessary and does not fix the real problem. I tracked the NaN problem down to src/html/layout.js line 277: if(typeof args.width != undefined){ should be if(typeof args.width != "undefined"){ Note the quotes. Same problem with lines 281, 301, 306
comment:9 Changed 16 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
(In [5535]) Fixes #894 (thanks [email protected]…)
comment:10 Changed 16 years ago by
Component: | General → Widgets |
---|---|
Milestone: | 0.4 → 0.5 |
Resolution: | fixed |
severity: | normal → minor |
Status: | closed → reopened |
Type: | defect → enhancement |
It seems that there were two unrelated issues in this ticket. The first issue relating to the NaN appears to have been fixed. sgendler, please verify and open a new ticket with a test case referencing this one if it is not. The second issue was an enhancement request on the Accordion widget, so I'm moving the milestone back to 0.5 and assigning back to Bill. The patch is not in a state which can be directly integrated into dojo.widget.*, so I'll leave it to Bill to decide if we should proceed.
comment:11 Changed 16 years ago by
Owner: | changed from Adam Peller to bill |
---|---|
Status: | reopened → new |
Summary: | [patch][cla]AccordionPane can't handle being created with display:none in Safari → [patch][cla]Accordion enhancement to allow setting of panel size (bug portion of this ticket already fixed) |
comment:12 Changed 15 years ago by
Component: | Widgets → Dijit |
---|---|
Milestone: | 0.9 |
Koranteng, sorry this one got stale. Could you please evaluate?
comment:13 Changed 15 years ago by
Owner: | changed from bill to Adam Peller |
---|
comment:14 Changed 15 years ago by
Resolution: | → invalid |
---|---|
Status: | new → closed |
Sorry again that we let this get stale. I think the problems have largely been addressed prior to the 0.9, and hopefully even more so now with the Dijit rewrite.
Bill, you might want to take a look at the patch I submitted in bug 893, as it is related to this one. That's what I am referring to in the first sentence of my description.