Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[4.0] Problems with class attribut in Field Switcher #19943

Closed
astridx opened this issue Mar 19, 2018 · 8 comments
Closed

[4.0] Problems with class attribut in Field Switcher #19943

astridx opened this issue Mar 19, 2018 · 8 comments

Comments

@astridx
Copy link
Contributor

astridx commented Mar 19, 2018

Steps to reproduce the issue

Create a field of type radio with the class switcher:

For example:

		<field
			name="show_title"
			type="radio"
			label="JGLOBAL_SHOW_TITLE_LABEL"
			class="switcher btn-group btn-group-yesno "
			default="1"
			>
			<option value="0">JHIDE</option>
			<option value="1">JSHOW</option>
		</field>

Expected result

The switcher should behave in Joomla 3 and Joomla 4 like expected. In Joomla 3 you need the classes btn-group btn-group-yesno and for Joomla 4 the class switcher is important.

Actual result

In Joomla 4 you do not see the green color if active.

Additional Informations

I think the problem is this line:
$type = str_replace('switcher switcher-', '', trim($class));

You can find this line here:

$type = str_replace('switcher switcher-', '', trim($class));

The type of the field is set with the class attribute.

I changed this line in

if (strpos($type, 'switcher') !== false){
	$type = 'switcher';
}

and everything works fine. My changes make, that if the word switcher is in the values of the class attribute, the type is set to switcher. But I am sure that this is not that easy ....

@dgrammatiko
Copy link
Contributor

@astridx there was some discussion about this in the other PRs. I think someone very brave needs to change the field to use the type attribute from the XML and also patch all the XMLs that have a switcher (that's the tricky part 😜)

@Bakual
Copy link
Contributor

Bakual commented Mar 20, 2018

Actually, the type attribute would be wrong. At least the way the field currently works (with two different JLayouts). Using the type attribute would mean we create a completely new field for that switcher.
We can do that of course, but then we have two fields which both are radios just with different appearance.
The current approach of switching the layout based on a CSS class is bad design.
The most correct way would be to set the layout in the XML to the switcher one. But that means adding that layout to each XML definition.
An alternative may be to add a new attribute like switcher="true" which then sets the layout accordingly. But that as well means to add a line to each switcher radio.

@C-Lodder
Copy link
Member

@Bakual It should be a new field ideally. Reason being is the radio type can have more than 2 options, whereas the switcher must have 2 only.

I agree that using a class to call the switcher is bad, however using the following is also bad:

<field
    type="radio"
    switcher="true"
/>

Much better to separate it and use:

<field
    type="switcher"
    class="primary" << // (success, danger, primary) (defaults to 'success')
/>

@brianteeman
Copy link
Contributor

Happy to do the leg work of patching the xml when a decision is made

@Bakual
Copy link
Contributor

Bakual commented Mar 20, 2018

With current code, the correct way to set the layout would be

<field
    type="radio"
    layout="joomla/form/field/radio/switcher"
/>

That does work without any changes in the PHP side.

However it doesn't solve the issue here (an own type doesn't solve it as well). The problem we have is that in J3 core uses button groups and in J4 we use switchers. If an extension now wants to work both in J3 and J4 and use the same UI which core uses, it isn't possible currently.
There are two issues there: One is that the XML changed (for now class attribute and you can't have both variants active) and the second is that the order of the options got changed. I don't see how that can both be fixed and I'm not sure if it even should be fixed.

As it is, the only option for now is to keep using button groups. Those are supposed to work both in J3 and J4. An extension can switch to using switchers only when they drop support for J3.

@Bakual
Copy link
Contributor

Bakual commented Mar 20, 2018

Have a look at #19953 if that fixes it. Imho it should.
My other PR (#19946) would also fix the btn-groups (again).

@ghost
Copy link

ghost commented Mar 20, 2018

closed as having Pull Request #19953

@joomla-cms-bot
Copy link

Set to "closed" on behalf of @franz-wohlkoenig by The JTracker Application at issues.joomla.org/joomla-cms/19943

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants