Package home | Report new bug | New search | Development Roadmap Status: Open | Feedback | All | Closed Since Version 0.6.2

Request #8241 Closes fieldset when it is not open.
Submitted: 2006-07-18 16:27 UTC
From: saltybeagle Assigned: wiesemann
Status: Closed Package: HTML_QuickForm_Renderer_Tableless (version 0.3.1)
PHP Version: 5.1.4 OS: N/A
Roadmaps: (Not assigned)    

 [2006-07-18 16:27 UTC] saltybeagle (Brett Bieber)
Description: ------------ The code which was in 0.3.0 which checked first before closing a fieldset was removed in 0.3.1. This affects people extending the tableless renderer, specifically using the same renderer for a nested form displayed within a qfElement. IE Justin Patrin's SubForm QuickForm element. Test script: --------------- --- /Users/bbieber/Desktop/HTML_QuickForm_Renderer_Tableless-0.3.1/HTML_QuickForm_Renderer_Tableless-0.3.1/HTML/QuickForm/Renderer/Tableless.php 2006-07-18 09:25:45.000000000 -0500 +++ Tableless.php 2006-07-18 11:19:41.000000000 -0500 @@ -142,7 +142,9 @@ } else { $header_html = str_replace('{header}', $header->toHtml(), $this->_headerTemplate); } - $this->_html .= $this->_closeFieldsetTemplate; + if ($this->_fieldsetIsOpen) { + $this->_html .= $this->_closeFieldsetTemplate; + } $openFieldsetTemplate = str_replace('{id}', $id, $this->_openFieldsetTemplate); $this->_html .= $openFieldsetTemplate . $header_html; $this->_fieldsetIsOpen = true;


 [2006-07-18 18:51 UTC] wiesemann (Mark Wiesemann)
Hi Brett, is this everything that needs to be changed? I saw your email to Justin, maybe there is more that I can help with in my renderer? Regards and thanks, Mark
 [2006-07-20 04:07 UTC] brett dot bieber+1 at gmail dot com (Brett Bieber)
I worked up a simple example with a subform... and an attempt at using a stack to remember the open elements. (I haven't tested this thoroughly).
 [2006-07-20 20:39 UTC] wiesemann (Mark Wiesemann)
> I worked up a simple example with a subform... and an > attempt at using a stack to remember the open elements. Hmm, does this really help you. Your stack usage example right on the hand, but the fieldset inside the div for the element looks a little bit strange? Maybe this whole subform thing is not really usable with this renderer. Justin wrote a table element for QF, maybe that is more suitable. This last comment does not mean that I want to decline anything, I'm just unsure about it. If you say, that it is usable in this way (or with some more changes from you), then I'm willing to update the code. Thanks so far.
 [2006-07-21 01:43 UTC] brett dot bieber+1 at gmail dot com (Brett Bieber)
Hey Mark... the CVS version works fine. 0.3.0 needed a bit of work, but with 0.3.1 everything is solved without having to use a stack... or modifying the subform element to reset fieldsetIsOpen. The renderer is very usable with the subform element, and it works quite well... a nested fieldset is easily hidden and expanded with some javascript to collapse sections of a form you don't always need. You've got my approval, it works great.
 [2006-07-21 01:59 UTC] brett dot bieber+1 at gmail dot com (Brett Bieber)
Ok, maybe one small correction. The CVS revision 1.6 works only if the subform has a header element in it... if I remove the subform's header element (see the fieldset is closed twice when it was only opened once. This IS a special case of this renderer with a non-standard quickform element... so you shouldn't have to worry about it. I'm just trying to work around it becuase I want to use your renderer in conjunction with some complex formbuilder forms.
 [2006-07-21 16:12 UTC] wiesemann (Mark Wiesemann)
Hi Brett, I'm not fully sure about the current status: Is the current CVS version (that only has the new if around the closing fielset call) working for you? (Apart from the case, where a header is missing, but that's a really special case.) Otherwise, please tell me, what's missing. Especially: Is your stack idea needed or isn't it needed? Thanks, Mark
 [2006-07-25 13:48 UTC] brett dot bieber+1 at gmail dot com (Brett Bieber)
Mark, "Is the current CVS version (that only has the new if around the closing fielset call) working for you? (Apart from the case, where a header is missing" Yep! Revision 1.6 in CVS works as you describe. If you have any questions, just try the example source on my pages out for yourself. I do think using a stack is helpful because this prevents the special case you refer to from happening. My code may not be the best, but the concept is (I think) essential in something like this... to remember what elements of the form were opened by pushing and popping a stack, because a boolean flag is not enough. The entire source of my stack sample and the modified source of your renderer is at For the purposes I'm using your renderer for I've modified Justin's subform class to handle this and can use a specific version of the renderer. So I'm fine with everything as is... I would recommend you implement the stack if you think it is useful and maintainable by you - and if you want to take care of people using FormBuilder & subforms. (if anyone else is effected by this - I suggest you speak up). -Brett
 [2006-07-26 19:17 UTC] wiesemann (Mark Wiesemann)
Hi Brett, can you please send me either the edited source file or a diff against it? The output on your page is unfortunately not really diffable (?) for me. Thanks, Mark
 [2006-08-01 19:06 UTC] wiesemann (Mark Wiesemann)
Anybody out there? In your online code, I couldn't find anything about the stack. Maybe I did not search clearly enough, but maybe it is really not there. Please send me the source file and/or a diff against current CVS code with the stack code implemented. Thanks, Mark
 [2006-08-05 02:55 UTC] brett dot bieber+1 at gmail dot com (Brett Bieber)
Mark! Sorry for the delay... just back from a conference and catching up on things. I accidently overwrote my stack code while I was testing. I've placed it back up on my server at: And the diff: Keep in mind I haven't tested this in too many cases. Thanks for your patience.
 [2006-08-05 21:38 UTC] wiesemann (Mark Wiesemann)
Hi Brett, thanks for the diff. I think that it should work this way, for both "normal" and "not normal" *g* forms. But I'm unsure about using an array for the stack. On the hand, the array handling with the push and pop calls, is of course the stack way. But wouldn't on the other hand a simple integer variable be also sufficient? Just something like $_fieldsetsOpen? (only one new letter) What do you think? Although I need to find a decision at the end, I'd really like to hear your opinion on this. Regards, Mark
 [2006-08-05 21:40 UTC] wiesemann (Mark Wiesemann)
Maybe some more words on the integer variable: The idea is to add 1, if a new fieldset is opened, and subtracting 1, if a fieldset is closed. The checks would be similar, like "if ($this_>_fieldsetsOpen) > 0" or something like that.
 [2006-08-05 22:45 UTC] brett dot bieber+1 at gmail dot com (Brett Bieber)
Integer is fine... I initially thought of using text inside the array to distinguish between an open form, qfelement, fieldset etc. I didn't add in any code for that... but that was the idea. Some examples beyond the fieldset issue are, in a nested subform if the <form> is open already within the stack, know to not open it again... if you are in a qfelement, the qfelement must be closed before an outer fieldset is closed, or a group etc. I don't know if those are needed, and I didn't add in any checks for those cases... I just know Justin had to code around some of that because the default renderer wanted to open a form tag with the nested form (for example). That's why I was thinking of using text.... but you could easily use integer constants for the output stack as well. This is probably overkill for just the small issue we're having here... but that was the best solution for all the isOpen/open/close issues my little brain could contemplate.
 [2006-08-06 13:04 UTC] wiesemann (Mark Wiesemann)
Okay, I've use the integer variable. If needed, this can be changed/extended later, but as far as I could follow you, this should be sufficient for now. A new release will be available shortly.