? ? Pending

User tests: Successful: Unsuccessful:

avatar joomla-ua
joomla-ua
30 Apr 2017

Pull Request for Issue # .

Summary of Changes

Fix form-horizontal for non-latin or long language strings.

Please look for this languages specifies fix: @infograf768

Before fix:
before

After fix:
screenshot-lang j-2017-04-30-04-20-12

Optional width 200px:

max-width: 200px;
min-width: 200px;
width: 200px;

Testing latest version Firefox, Chrome and etc Chromius based brawsers.

avatar joomla-ua joomla-ua - open - 30 Apr 2017
avatar joomla-ua joomla-ua - change - 30 Apr 2017
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 30 Apr 2017
Category Administration Templates (admin)
avatar CB9TOIIIA
CB9TOIIIA - comment - 30 Apr 2017

Joomla pain :)

avatar b2z
b2z - comment - 30 Apr 2017

As a Russian language user I am definitely for it. Looking much better now.

avatar Arkadiy-Sedelnikov
Arkadiy-Sedelnikov - comment - 30 Apr 2017

Agree

avatar franz-wohlkoenig
franz-wohlkoenig - comment - 30 Apr 2017

if 2 People test this PR successfully it could go in staging if Maintainers agree.

avatar ot2sen
ot2sen - comment - 30 Apr 2017

I have tested this item successfully on cc318d8

Tested with multiple languages and Russian in particular which had this issue.
Logic improvement on i18n to ensure any language looks good/correct in the interface for admin forms.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/15689.
avatar ot2sen ot2sen - test_item - 30 Apr 2017 - Tested successfully
avatar brianteeman
brianteeman - comment - 30 Apr 2017

Please do not edit the css file directly. That is not correct.

The correct way is to make the changes in the less file and then run generatecss.php from the build directory to create the css

avatar infograf768
infograf768 - comment - 1 May 2017

Unhappily, we have multiple .form-horizontal .control-label in the less => css files.
Some have width: auto; depending on the screen width.

Therefore the less file to modify has to be chosen carefully
Also nothing is changed for rtl here.

avatar infograf768
infograf768 - comment - 1 May 2017

@ciar4n
Can you have a look?

avatar ciar4n
ciar4n - comment - 1 May 2017

The Bootstrap default is for the control-label to wrap at 160px so if the intent is to allow the label to wrap then simply removing https://github.com/joomla/joomla-cms/blob/staging/administrator/templates/isis/less/blocks/_forms.less#L10 should give the same result. If you wish to increase the label width you will also need to increase the left margin on .form-horizontal .controls by the same amount. Not sure if there is a need for setting the min and max width.. maybe you have your reasons?

Test RTL however it should be ok. Media queries should also be fine as is.. smaller screen sizes have the label and control in columns.

As mentioned by @brianteeman you will need to edit the LESS files and then compile rather than editing the CSS directly (https://docs.joomla.org/Joomla_LESS).

avatar joomla-ua joomla-ua - change - 3 May 2017
Labels Added: ?
avatar Fedik
Fedik - comment - 3 May 2017

I have tested this item successfully on 6250e2f


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/15689.

avatar Fedik Fedik - test_item - 3 May 2017 - Tested successfully
avatar Quy
Quy - comment - 3 May 2017

It looks good. The only 1 display issue is under System > Global Configuration > Media that just needs markup change.

Before:
media-before

After:
media-after

avatar brianteeman
brianteeman - comment - 3 May 2017

@quy that might happen in other places too especially with extensions

avatar Fedik
Fedik - comment - 3 May 2017

it because a Spacer field displaying the text as a label, this is some very old legacy.
But even with this, it is looks better than on the screenshot from the pull description

avatar Fedik
Fedik - comment - 3 May 2017

upd: In theory, for a Spacer field we can force class in "JFormFieldSpacer::renderField", something like field-spacer and then in css use .field-spacer>.control-label{width:auto;}
@quy what do you think?

avatar Quy
Quy - comment - 3 May 2017

Sounds good to me in theory. but I am no css guru. :)

avatar joomla-ua
joomla-ua - comment - 3 May 2017

@Fedik it's nice idea!

avatar Fedik
Fedik - comment - 4 May 2017

@joomla-ua you will update the PR?

avatar infograf768
infograf768 - comment - 5 May 2017

@Fedik
Not sure where to add the class in the library.

avatar Fedik
Fedik - comment - 5 May 2017

@infograf768
in JFormFieldSpacer class, add method

public function renderField($options = array())
{
   $options['class'] = empty($options['class']) ? 'field-spacer' : $options['class'] .  ' field-spacer';
   
   return parent::renderField($options);
}

other solution can be, to add a layout (renderLayout) specially for a Spacer field instead of default joomla.form.renderfield

avatar dgt41
dgt41 - comment - 5 May 2017

I like the layout approach more

avatar Fedik
Fedik - comment - 5 May 2017

@dgt41 yeah, but how it should looks like? for not break current markup of the forms ?

avatar infograf768
infograf768 - comment - 6 May 2017

I can't get it to work here
we have to overwrite width: 160px; in

.form-horizontal .control-label {
    float: left;
    padding-top: 5px;
    width: 160px; 
}

and the code above does not.

avatar Fedik
Fedik - comment - 6 May 2017

@infograf768 I have sent pull with updates to @joomla-ua
the configuration page render the fields without use of renderField, that why the code from my previous comment not worked ?

@dgt41 I think here we can use the class, because the layout it is a "new feature" and cannot be accepted until j3.8, I guess.
We can do the layout for Spacer field anytime later.

avatar infograf768
infograf768 - comment - 6 May 2017

Eager to test. We have multiple types of spacers and this should take into account all of them.
I would not consider this such a new feature that it could not go into a 3.7.2 (if no B/C impact on frontend).

avatar joomla-cms-bot joomla-cms-bot - change - 6 May 2017
Category Administration Templates (admin) Administration com_config Templates (admin) Libraries
avatar Quy
Quy - comment - 6 May 2017

It works. I just noticed under System > Global Configuration > Templates, the following does not require the spacer pr because it is coded differently. Should System > Global Configuration > Media be changed to match?

Supported File Formats
Be careful before changing the file types. Read the tool tips before editing. 
avatar Fedik
Fedik - comment - 6 May 2017

@Quy that another field type note, that actually use a hack to be out of the ".control-label" ?
https://github.com/joomla/joomla-cms/blob/staging/libraries/joomla/form/fields/note.php#L60

avatar infograf768
infograf768 - comment - 6 May 2017

Will test tomorrow. ?

avatar infograf768
infograf768 - comment - 7 May 2017

I have tested this item successfully on 21929d6

Thanks. Works great.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/15689.
avatar infograf768 infograf768 - test_item - 7 May 2017 - Tested successfully
avatar infograf768
infograf768 - comment - 7 May 2017

@Fedik
We forgot something.
you should do the same for /layouts/joomla/content/options_default.php to cope with Global Configuration.

For example, I have added in /administrator/components/com_config/model/form/application.xml a new spacer field.

                <field name="spacer1"
			type="spacer"
			class="text"
			label="COM_CONFIG_FIELD_DEFAULT_CAPTCHA_DESC"
		/>

(I used an existing lang string).
And I get this
screen shot 2017-05-07 at 08 48 57

Should be easy

avatar infograf768
infograf768 - comment - 7 May 2017

diff would be:

diff --git a/layouts/joomla/content/options_default.php b/layouts/joomla/content/options_default.php
index 9b42831..e36532c 100644
--- a/layouts/joomla/content/options_default.php
+++ b/layouts/joomla/content/options_default.php
@@ -26,4 +26,5 @@
 		{
 			$datashowon = '';
+			$groupClass = $field->type === 'Spacer' ? ' field-spacer' : '';
 
 			if ($field->showon)
@@ -34,5 +35,5 @@
 			}
 			?>
-			<div class="control-group"<?php echo $datashowon; ?>>
+			<div class="control-group<?php echo $groupClass; ?>"<?php echo $datashowon; ?>>
 				<?php if (!isset($displayData->showlabel) || $displayData->showlabel) : ?>
 					<div class="control-label"><?php echo $field->label; ?></div>
avatar joomla-cms-bot joomla-cms-bot - change - 7 May 2017
Category Administration Templates (admin) com_config Libraries Administration com_config Templates (admin) Layout Libraries
avatar infograf768
infograf768 - comment - 7 May 2017

I have tested this item successfully on d0e5e57


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/15689.

avatar infograf768 infograf768 - test_item - 7 May 2017 - Tested successfully
avatar infograf768
infograf768 - comment - 7 May 2017

@ot2sen
Please test gain to get this one in.

avatar AlexRed
AlexRed - comment - 7 May 2017

I have tested this item successfully on d0e5e57

Patch ok for me


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/15689.

avatar AlexRed AlexRed - test_item - 7 May 2017 - Tested successfully
avatar ot2sen
ot2sen - comment - 7 May 2017

I have tested this item successfully on d0e5e57


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/15689.

avatar ot2sen ot2sen - test_item - 7 May 2017 - Tested successfully
avatar franz-wohlkoenig franz-wohlkoenig - change - 7 May 2017
Status Pending Ready to Commit
avatar franz-wohlkoenig
franz-wohlkoenig - comment - 7 May 2017

RTC after two successful tests.

avatar infograf768
infograf768 - comment - 7 May 2017

@rdeutz
No need to wait 3.7.2 for that one. Can be merged imho for 3.7.1

avatar rdeutz
rdeutz - comment - 7 May 2017

I will keep 3.7.1 as light as possible, so if it is not without any doubt a bugfix it has to wait for 3.7.2

avatar infograf768
infograf768 - comment - 7 May 2017

OK, no p

avatar rdeutz rdeutz - change - 22 May 2017
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2017-05-22 19:15:34
Closed_By rdeutz
Labels Added: ?
avatar rdeutz rdeutz - close - 22 May 2017
avatar rdeutz rdeutz - merge - 22 May 2017

Add a Comment

Login with GitHub to post a comment