1
goffy
set class for form elements
  • 2018/7/7 10:20

  • goffy

  • Just can't stay away

  • Posts: 535

  • Since: 2010/12/27


hi

is it right that a form element has the functions setClass and getClass, but they are not used for rendering a form element?

2
zyspec
Re: set class for form elements
  • 2018/7/10 14:56

  • zyspec

  • Module Developer

  • Posts: 1095

  • Since: 2004/9/21


@goffy,

If I understand your question you want to use the XoopsForm element classes to instantiate and then render the element, correct?

If so, yes you can do that. You can do something like:
$myRadioElement = new \XoopsFormRadioYN('Is the weather nice today?''ynradio');
echo 
$myRadioElement->render();


This will render a simple Yes/No radio element. Of course you can also "capture" the output in a variable and then display it later if you want too:
$myRadioElement = new \XoopsFormRadioYN('Is the weather nice today?''ynradio');
$radioHTML $myRadioElement->render();
... <
more code here> ...
echo 
$radioHTML;

3
goffy
Re: set class for form elements
  • 2018/7/10 22:18

  • goffy

  • Just can't stay away

  • Posts: 535

  • Since: 2010/12/27


hi zyspec

i want to use the class during rendering

I have adopted my xoops e.g.
public function renderFormButton(XoopsFormButton $element)
    {
        if ( 
$element->getClass() ) {
            return 
"<input type='" $element->getType() . "' class='" $element->getClass() . "' name='"
            
$element->getName() . "'  id='" $element->getName() . "' value='" $element->getValue()
            . 
"' title='" $element->getValue() . "'" $element->getExtra() . ' />';
        } else {
            return 
"<input type='" $element->getType() . "' class='btn btn-default' name='"
            
$element->getName() . "'  id='" $element->getName() . "' value='" $element->getValue()
            . 
"' title='" $element->getValue() . "'" $element->getExtra() . ' />';
        }
    }


now I can use the function.

My question is: I think it make sense. Should I make these changes in xoops core and send a request? or is there another idea?

4
Mamba
Re: set class for form elements
  • 2018/7/12 2:20

  • Mamba

  • Moderator

  • Posts: 11366

  • Since: 2004/4/23


What would be "use case" for it?

As I understand, the XoopsFormRenderer is basically a "factory" to create a consistent visual implementation for all elements of the XoopsForm

In XOOPS 2.5.9 Mage and Geekwright extracted it to an Interface, and added the Bootstrap implementation, so now you can call either the XoopsFormRendererLegacy or XoopsFormRendererBootstrap3.
And you could provide XoopsFormRendererXXX that would be specific to your "look & feel" by using some other CSS framework.

It's along the line: "Don't call us, we'll call you", i.e. you provide the implementation via XoopsFormRendererXXX , and XOOPS will call it and use it to render individual elements of your implementation as needed.

More about the Hollywood Principle using the Template Method:

Resized Image


So if I understand correctly, instead of going trough the XoopsFormRendererXXX and let it do the job for you, you would like to be able to provide implementation for individual element (like the FormButton) and then render it individually as needed, correct?

What would be the advantage of it?

Do you want to have an option that in case you don't like the implementation of XoopsFormRendererBootstrap3, instead of modifying the whole Renderer, you just want the option to change the FormButton? If yes, could you just subclass the Renderer and make the changes as needed there?

BTW - there was an interesting article about using "Inheritance of XoopsForms"

Resized Image
Support XOOPS => DONATE
Use 2.5.10 | Docs | Modules | Bugs

5
goffy
Re: set class for form elements
  • 2018/7/16 11:39

  • goffy

  • Just can't stay away

  • Posts: 535

  • Since: 2010/12/27


hi Mamba

I want to use full possibilities of bootstrap, nothing instead

example:
by using XoopsFormButtonTray I get following buttons:
- Cancel ( bootstrap class: btn-danger = red)
- Reset ( bootstrap class: btn-warning = yellow)
- Submit ( bootstrap class: btn-success = green)

If I now want to add a fourth button this button (as a standard button) will have bootstrap class: btn-default (= no color)

with my suggested changes I can decide, which style my additional button will have

see image Resized Image

6
zyspec
Re: set class for form elements
  • 2018/7/16 16:37

  • zyspec

  • Module Developer

  • Posts: 1095

  • Since: 2004/9/21


@goffy,

I apologize for misunderstanding your original request - I think I do now. You are correct, unfortunately neither the XoopsFormRenderLegacy or the XoopsFormRenderBootstrap3 classes render the Form Button Element's class information. It looks like 'renderFormButton' method should be modified similarly to what you've done in both of these classes. The only difference between the Legacy and Bootstrap3 class methods would be the default classes rendered if the getClass() method returns false.

It looks like this is very similar to what has been done already in XOOPS 2.6 so anything written to utilize your proposed change for 2.5.x should behave similarly in 2.6.

There are several other element types which will have the same issue (Radio, Button Tray, Checkbox, etc.) so UNLESS they're all going to be changed I wouldn't recommend this for the 2.5.x core. Having these elements behave differently from each other (ie. you can't count on the behavior) then I don't think there's enough value in XOOPS 2.5.x to make this change. If someone's willing to change it virtually "everywhere" in 2.5.x rendering engines then it would be a valuable addition.

Just my 2 cents.

But yes, I would be a proponent of changing it "everywhere" in 2.5.x if someone has the time to code/test it - I'm just not sure the current core team has the bandwidth to provide us that luxury.

[EDIT]
One short term "hack" is to use the setExtra() method to set the additional class information since the "extra" information is rendered. Using this method would then give you a button with multiple separate 'class' attributes but it might be a better solution than nothing. Most modern browsers will render this as expected. So you would end up with a button rendering that looks something like:
<input type='button' class='btn btn-default' name='myButton'  id='myButton' value='Submit and go to images upload' title='Submit and go to images upload' class='btn-success' />';

While not ideal you could do something similar to this for each HTML element type you use (button, radio, etc).

Just an idea
[/EDIT]

7
geekwright
Re: set class for form elements

Let's roll this back a little bit, and examine what has changed and why.

As we worked on 2.5.9, a lot of effort when in to standardizing the display of system generated elements, and a special emphasis was given to Bootstrap based themes. Older modules, designed and coded long before there was a bootstrap framework, generally looked terrible. To be blunt, it generally looked like crap. The most common reason was due to bringing in CSS that was incompatible, causing serious visual breakage.

Newer modules, designed with bootstrap in mind, looked OK, but many themes came with a set of hacks to apply to the system and modules to fix some problems. Templates manipulated generated HTML to change elements, as well as add or remove attributes to make certain modules look better.

The problem was, not each theme had the same set of changes. So, each time a module was updated, different themes would break. If a module was reworked to look good under bootstrap, it broke when used with older themes.

The big problem, when we looked at it, was that PHP code (domain logic) was setting style attributes (presentation details,) and there was no way to override PHP code by theme. If it is set in the PHP code side, there is no way to know how the theme will interpret it. Some frameworks, such as Bootstrap, feature semantic classes rather than just visual classes, and that is good, but those semantics are unique to Bootstrap, and even specific versions of Bootstrap. Further, while Bootstrap enjoys wide adoption, it is far from the only scaffolding, and many find fault with it. If we code core to Bootstrap, we severely limit ourselves.

XOOPS inherently has a separation between logic, data and presentation. The Smarty template engine makes it possible to build a set of data in a PHP program that is passed to the engine, where the set of data is merged with a set of templates that control how the data is presented, and how it looks. XOOPS provides for themes which can override the templates provided by the module.

There are, unfortunately, places where PHP code belches out raw HTML, and thus becomes involved in the visual presentation aspects. The biggest example of those was the form elements. All the form element rendering was stripped out into classes implementing the \XoopsFormRendererInterface. The exact renderer used can be controlled by or overridden by the theme by specifying a theme_autorun.php file.

To achieve the goal of a consistent look and feel it was necessary to ignore the class attributes, managed by the FormElement::setClass() method. The existence of that method violates that separation between logic and presentation. It might be possible to whitelist a set of allowable classes, but to make that work across all themes, we would have to dictate our own intermediate set of semantic classes, and translate those in the renderer. Just to understand the scope of that effor, Bootstrap is now a version 4 of their semantics, and it is still evolving. The setClass() and getClass() methods are relics, and should have been marked a deprecated. They open a set of problems that cannot be fixed.

But, that doesn't mean all hope is gone. The solution to the missing setClass() is simple -- do it in the theme!

There should be a template where the form is displayed. Within that template, you can address each form element by its ID, so a script section can easily set any desired attribute, including class. Add a comment or two about why these are needed, and you make a template that others can easily adapt for any CSS framework. And it is all in the control of the theme, not some chunk of PHP code that needs to be hacked again and again over time.

It is just a growing pain.

8
goffy
Re: set class for form elements
  • 2018/7/17 5:47

  • goffy

  • Just can't stay away

  • Posts: 535

  • Since: 2010/12/27


hi geekwright

I agree, partially.
The separation between logic, data and presentation should be anyway, and I code no presentation with php in my modules.
My idea was to get the same look and feel - each submit button should have the same style (btn-success in this case).

And if we remove style code 100% from XOOPS core then we have to remove all class attributes in XoopsFormRendererBootstrap3.php
But then it doesn't look like bootstrap anymore

9
geekwright
Re: set class for form elements

\XoopsFormRendererInterface defines the interaction between the business logic supplied by form elements and the presentation layer rendering of those elements. All HTML generated by core form and form element classes is done by an implementation of \XoopsFormRendererInterface.

\XoopsFormRendererBootstrap3 is a component supplied with core that provides a concrete implementation of \XoopsFormRendererInterface. A theme may optionally use that component. If the theme does not explicitly specify an implementation of \XoopsFormRendererInterface to use, the rendering choice will default to \XoopsFormRendererLegacy, which closely mimics the HTML generated in pre-2.5.9 form elements.

The separation through the interface makes the HTML generation the responsibility of the presentation layer, more specifically the theme. New and different renders are possible and expected, even for frameworks that have not been written yet.

To complete the separation, we need to stop passing the presentation detail directly from the business logic. This kind of detail belongs in the template where it can be adjusted by theme.

$( document ).ready(function() {
    $(
'#myspecialbutton').addClass('btn-success').removeClass('btn-default');
});

Login

Who's Online

237 user(s) are online (150 user(s) are browsing Support Forums)


Members: 0


Guests: 237


more...

Donat-O-Meter

Stats
Goal: $100.00
Due Date: Mar 31
Gross Amount: $0.00
Net Balance: $0.00
Left to go: $100.00
Make donations with PayPal!

Latest GitHub Commits