QControl.class.php and QcontrolBase.class.php is problematic...

[kukrik]

With Qcontrol.class.php you have problems that clearly suggest to the part of QcontrolBase?.class.php bInUseWrapper, that does not work correctly. If you put on false.

In case of $this->blnUseWrapper=true;everything works correctly.

The problem is discussed a lot in this following forum:  http://qcu.be/content/qcontrolclassphp-problematic#comment-7384

[vakopian]

I have seen this problem as well. So just to explain the problem a little: the developer sets UseWrapper to false, then calls RenderWithName. Without the patch in #824, the label for the controls appears multiple times (one more time with every ajax refresh). With the patch in #824, this problem is fixed, however, the control internally sets blnUseWrapper to true, and thus brings back the wrapper, against the developers wishes and expectations.

To test, you can use the /assets/_core/php/examples/events_actions/delayed.php example by setting blnUseWrapper (and by rolling back the patch from #824).

The reason for the bug is explained by Mike in the forum thread: normally, the ajax calls update the wrapper control. But when there is no wrapper, the ajax call has to update the control itself, which doesn't include the label. So when the ajax update comes back with the the full control (including the label), we get two labels - the old one, and the new one.

One solution could be to not render the name when the call type is ajax (e.g. by checking check the Form->strCallType == QCallType::Ajax). The problem with this is that if the control's Name was modified by the ajax action, it will not be reflected.

Another solution could be for the ajax call to remove the old label (and any other "auxiliary" elements) before updating the control. If doable this would be a better solution.

Mike, what do you think?

[mikederfler]

I think I have a simple solution for the problem:
Instead of adding an id to the name,error ... elements
just add a property "data-rel='id_of_related_control'" to them.
When performing an Ajax update and the control that is updated is wrapper less
remove all siblings that have the attribute "data-rel='id_of_related_control'".

So it is possible to add any extra custom stuff as long as the element that is a sibbling of the control to be updated has the attribute "data-rel='id_of_related_control'" set.

Example: when updating the input element the error span and the label get removed (and added again if necessary)

<label for="mycontrol" data-rel="mycontrol"
<input id="mycontrol"/>
<span data-rel='mycontrol' class="error">
   <p>A complex error message ... could have images or anything inside</p>
   <img src="smiley.gif" alt="Smiley face" height="42" width="42" />
</span>

[kukrik]

I think that vakopian is right when he referred to the upper half of the problem ... I know very well that in "base_controls" files are not absolutely desirable to change.

Testing and investigation took QcontrolBase?.class.php lines 1028 - 1030 and 1052 - 1054 wrappers (div and span) out. I left a $ this-> blnUseWrapper = true; inside, it should not be such a problem to come ... But came to the same problems that I have described at length forum .... At the same time, and I know I've done so before doing any code, you testin without wrappers, if it is working as it should be, it will come in later wrappers, etc. ... Here is the opposite - wrappers conceal faulty code, if I'm not mistaken?!

In the past it not caused me a headache wrappers, but now I have one client's offered an interesting call to build a web app for deskop, tablets and mobile device, which require pure HTML5, CSS3, jQuery etc.... I just try to implement onto QCubed, what i love to make. QControl.class rework is not a problem, the problem is the creation unnecessary wrapping QcontrolBase?.class by. Forms validation is extremely important for almost every application...

Question to mikederfler! I have a bit of a stranger use the attribute data-rel. I would ask for more specific example in advance, would be a good option, if you take for example QControl.class and doing your recommendation of this class around, then we can better understand. How does that sound?

[mikederfler]

So these changes are the implementation of the idea I mentioned before.
No more forcing of wrappers for controls rendering with error or name.
If a wrapper-less control is rendered with error or with name, the special attribute
data-rel="controlId" is added to the error/name control
Additionally if this control is a top level ajax response, the attribute data-hasrel
is added to the "control" (look at QControlBase::RenderOutput?). This is checked in javascript. If it is present
this control is searched in the dom and all its siblings that have the data-rel="controlId" attribute set are removed.

Important: this procedure has to be done only for the top level ajax request control.
That means there is minimal javascript overhead. You can have thousands of nested controls rendered with error but this procedure has to be performed only on the root
of them (the ajax top level response control)

Don't forget to uncomment

protected $blnUseWrapper = false

in QControl.class.php when testing

[vaibhav]

I am unable to use the patches. Looks like I ran PhpStorm's code formatter on entire codebase :(

Can you tell me revison number over which the patches were generated so I could test them locally by checking out the relevent revision only? Also, I do not understand what data-rel attribute actually does. I searched on Google and the answers seem confusing and vague and I did not get it properly. If possible, can you point me to some documentation about that attribute?

[mikederfler]

The patches are against trunk (2.1 svn head)

[vaibhav]

Ok checking out. How do I reproduce the problem which the patch solves? (so that I will first try to see if it actually works on my system as well or not).

[vaibhav]

Where is this line in the QControl.class.php file?

protected $blnUseWrapper = false;

I checked out the head and I just did not get it! Something wrong?

[kukrik]

I made this topic to address the problem, and I think I know better to try it. Vaibhav is a very good man, actually a nice guy :). I'm sorry, but I have not found patches (mikederfler: The patches are against trunk (2.1 svn head)). Please provide a link so that I can contribute a little bit to talk about ...

Digression want to ask you, Vaibhav on this topic, you wrote: http://trac.qcu.be/projects/qcubed/wiki/PhpStorm?version=1. I would also be very interested in PhpStorm program. But I'm not a core contributor, i well as part taking in this very small, since my main job is another. Soon I will start a 30-day period to expire PhpStorm ... I tried this and it started to like it. If no other way can not be free, I buy this program with my money.

Regards,

Tiit

[mikederfler]

Sorry for the late reply. I was moving to another town.
@kukrik:

You can check out the svn version and apply the patches.
Or should I post the whole files for simple testing.
You could use them as a drop in replacement (save your files;) )

[kukrik]

It's okay, I was also out of town for a few days :)! Just today I was back. I understand that the patch takes a lot of time copying in the right places ... Second, I use a Mac. Subversion not a free version for the Mac. Total payable :(!

The best way to get a complete three files to test them! How does that sound?

[mikederfler]

Added the 3 files for testing. Don't forget to ctrl+F5 --> the qcubed.js file has changed

[kukrik]

mikederfler!

Your posted code worked well, the source is net, it means that it is much less wrappers, it gives more flexibility to the designer ...

However the problem remains the same, validating forms creates a double label.

I would ask that if you are a tested and double-label does not come????

[mikederfler]

@kukrik:
Is the error message duplicated too?
Because this works for me.

I have to take a look at the RenderWithName? part.
There might be something missing.

[mikederfler]

Ok I have found the error:

With the new version you can put in any custom style of label / error displaying
as long as you:
* add "data-rel='#myRelatedControlId'" (replace myRelatedControlId with the id of the control this error message belongs to) to the top level error / label element(s).
* pass the string $strWrapperAttributes = "data-hasrel='1'"; to RenderOutput?

return $this->RenderOutput($strToReturn, $blnDisplayOutput,false,$strWrapperAttributes);

in your custom render method i.e. RenderWithNameCustomCss?()
--> look at QControl::RenderWithName? for an example usage

The control can even be wrapped with such a custom element.

examples of wrapper-less controls with label and error message:

<div data-rel="#c1" class="renderWithName">
   <div class="left required"><label for="c1">Value1</label></div>
   <div class="right">
       <input id="c1" type="text"/>
       <span class="error">Value1 is required</span>
   </div>
</div>

<label for="mycontrol" data-rel="#mycontrol"
<input id="mycontrol"/>
<span data-rel='#mycontrol' class="error">
   <p>A complex error message ... could have images or anything inside</p>
   <img src="smiley.gif" alt="Smiley face" height="42" width="42" />
</span>

Note:
If you use wrappers with your custom Render method you do not have to care about the "data-rel" attribute.

[kukrik]

Mike!

Now all working as expected :)! I use 2 for the app, the first app is a standard package, and the second application is used for development. Both work very well.

But your note: If you use a wrapper with your custom Render method you do not have to care about the "data-rel" attribute. This is not correct, try to cope without attributes, it did not work as they should be. Should be left in the property. But it does not bother me now, I get to move on...

You should definitely take the other test and give feedback ...

PS:. You am a talented boy or man :)

Thank you so much!

[mikederfler]

Glad to here that it works.

You are right about my previous note.
I should have mentioned that you do not have to care about the flags
if you set $this->blnUseWrapper = true in the custom render method

[vaibhav]

Should I then just apply the patch on my end?

BTW I am still not able to understand what error it solves :|

[kukrik]

Vaibhav!

There's no mistake, if you use the standard Render method. But if you want to take into account the wishes of the client, such as a client wants to build a web app deskop, tablet and mobile device, which require pure HTML5, CSS3, JQuery, etc. ..., then duplicate and unneeded wrappers do not let work properly ...

In comparison, two examples of the tone, when you look at the source of the text:

1) If you put inside QControl.class.php: <tt>protected blnUseWrapper $ = true;>/tt>

It is normal to see the code:

<tt<div id="c2_ctl" style="display:inline;">
<div class="renderWithName">

<div class="left required"> <Label for="c2"> First Name </ label> </ div>
<div class="right"> <input id="c2" class="textbox" type="text" value="" maxlength="50" name="c2"> </ div>

</ div>
</ div></tt>
2) If you leave this class: <tt>protected $ blnUseWrapper = false;</tt>
<tt>Then you have to see: <div class="renderWithName" data-rel="#c2">

<div class="left required"> <Label for="c2"> First Name </ label> </ div>
<div class="right"> <input id="c2" class="textbox" type="text" value="" maxlength="50" name="c2"> </ div>

</ div></tt>
3) I have a the custom render method thanks to mikederfler, which gives me a cleaner code:
<tt><p class="inline-large-label button-height" data-rel="#c2">

<Label class="label" for="c2"> First Name </ label>
<input id="c2" class="input" type="text" value="" maxlength="50" name="c2">

</ p></tt>
Just a cleaner code was me needed :). Remained unclear whether anything???

[kukrik]

Replying to kukrik:
Vaibhav!

There's no mistake, if you use the standard Render method. But if you want to take into account the wishes of the client, such as a client wants to build a web app deskop, tablet and mobile device, which require pure HTML5, CSS3, JQuery, etc. ..., then duplicate and unneeded wrappers do not let work properly ...

In comparison, two examples of the tone, when you look at the source of the text:

1) If you put inside QControl.class.php: protected blnUseWrapper $ = true;

It is normal to see the code:

<div id="c2_ctl" style="display:inline;">

<div class="renderWithName">

<div class="left required"><Label for="c2">First Name</label></div>
<div class="right"><input id="c2" class="textbox" type="text" value="" maxlength="50" name="c2"></div>

</div>

</div>

2) If you leave this class: protected $ blnUseWrapper = false;

Then you have to see:

<div class="renderWithName" data-rel="#c2">

<div class="left required"><Label for="c2">First Name</label></div>
<div class="right"><input id="c2" class="textbox" type="text" value="" maxlength="50" name="c2"></div>

</div>

3) I have a the custom render method thanks to mikederfler, which gives me a cleaner code:

<p class="inline-large-label button-height" data-rel="#c2">

<label class="label" for="c2">First Name</label><input id="c2" class="input" type="text" value="" maxlength="50" name="c2">

</p>

Just a cleaner code was me needed :). Remained unclear whether anything???

EDIT: I do not know how to use the wiki formatting. Sorry ...

[mikederfler]

@Vaibhav:
It makes it possible to create any custom render method (render with name and error ...) without wrapper.

Before this was not possible because the elements not being the control itself would have been duplicated on an Ajax call (sometimes).

Now it is possible to choose any form of css style for your custom render method regardless if it uses a wrapper or not.

i.e.: twitter bootstrap render with name and error

<div class="control-group warning" data-rel="#inputWarning">
  <label class="control-label" for="inputWarning">Input with warning</label>
  <div class="controls">
    <input type="text" id="inputWarning">
    <span class="help-inline">Something may have gone wrong</span>
  </div>
</div>

zurb foundation:

<label class="error" data-rel="#myinput">Error</label>
<input id="myinput" type="text" class="error">
<small class="error" data-rel="#myinput">Invalid entry</small>

note the data-rel attribute for the top level elements that are not the control (input) itself

[vakopian]

(In [1505]) fixes #859