? ? Success

User tests: Successful: Unsuccessful:

avatar dgt41
dgt41
23 Apr 2016

Separate Logic from output.
This is 100% B/C and allows templates to customise the core output of Joomla.
There shouldn't be any performance degradation since JLayouts are using caching.
It's a straight forward change, nothing magic here!

Response to the conversation: https://groups.google.com/forum/#!topic/joomla-dev-cms/I3onEgcwKZE

Notifying @chrisdavenport

Testing
Try to upload a file in media manager or install a component/module/plugin by selecting a zip file

569e7ee 23 Apr 2016 avatar dgt41 init
avatar dgt41 dgt41 - open - 23 Apr 2016
avatar dgt41 dgt41 - change - 23 Apr 2016
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 23 Apr 2016
Labels Added: ?
avatar brianteeman brianteeman - change - 23 Apr 2016
Category Fields Layout
avatar brianteeman brianteeman - test_item - 23 Apr 2016 - Tested successfully
avatar brianteeman
brianteeman - comment - 23 Apr 2016

I have tested this item :white_check_mark: successfully on 569e7ee


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

avatar joomla-cms-bot
joomla-cms-bot - comment - 23 Apr 2016

This PR has received new commits.

CC: @brianteeman


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

avatar andrepereiradasilva andrepereiradasilva - test_item - 23 Apr 2016 - Tested unsuccessfully
avatar andrepereiradasilva
andrepereiradasilva - comment - 23 Apr 2016

I have tested this item :red_circle: unsuccessfully on d53a5c9

Besides the Tste: i tested this by adding to one of the joomla xmls.

<field
    name="testfile"
    type="file"
    label="custom label"
    description="custom description"
    size="40"
    accept="image/*"
    class="test-class1 test-class2"
    disabled="true"
    multiple="true"
    autofocus="true"
    required="true"
    onchange="alert('a');"
/>

Result:

<input type="file"
    name="jform[testfile][]"
    id="jform_testfile"
     required aria-required="true" />


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

avatar joomla-cms-bot
joomla-cms-bot - comment - 23 Apr 2016

This PR has received new commits.

CC: @andrepereiradasilva, @brianteeman


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

avatar dgt41
dgt41 - comment - 23 Apr 2016

Oops, my bad! Now everything should be fine:
screen shot 2016-04-23 at 19 21 25

avatar andrepereiradasilva
andrepereiradasilva - comment - 23 Apr 2016

@dgt41 thanks it works. for me is tested with success.
I would do it with clean HTML output (without tabs and line ends), but is working fine.

Before patch:

<input type="file" name="jform[testfile][]" id="jform_testfile" accept="image/*" disabled class="test-class required" size="40" onchange="alert('a');" required aria-required="true" autofocus multiple />

After patch

<input type="file"
    name="jform[testfile][]"
    id="jform_testfile"
     size="40"   accept="image/*"    class="test-class required"     multiple    disabled    autofocus   onchange="alert('a');"  required aria-required="true" />
avatar andrepereiradasilva andrepereiradasilva - test_item - 23 Apr 2016 - Tested successfully
avatar andrepereiradasilva
andrepereiradasilva - comment - 23 Apr 2016

I have tested this item :white_check_mark: successfully on 4784cee


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

avatar chrisdavenport
chrisdavenport - comment - 23 Apr 2016

Looks good to me. I like it.

avatar smz smz - test_item - 23 Apr 2016 - Tested successfully
avatar smz
smz - comment - 23 Apr 2016

I have tested this item :white_check_mark: successfully on 4784cee

Way to go! :+1:

But... in the JLayout, why not just:

echo '<input type="file" name="' . $name . '" id="' . $id . '"';
echo !empty($size) ? ' size="' . $size . '"' : '';
echo !empty($accept) ? ' accept="' . $accept . '"' : '';
echo !empty($class) ? ' class="' . $class . '"' : '';
echo !empty($multiple) ? ' multiple' : '';
echo $disabled ? ' disabled' : '';
echo $autofocus ? ' autofocus' : '';
echo !empty($onchange) ? ' onchange="' . $onchange . '"' : '';
echo $required ? ' required aria-required="true"' : '';
echo '/>';

that is without all the opened-and-closed <?php ... ?>?
Using echo for the first part will also generate a tidier HTML without extra newlines and tabs...

Edit: or even a single multi-line echo with strings concatenation or commas (see: http://www.electrictoolbox.com/php-echo-commas-vs-concatenation/ if interested in very marginal optimization...)


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

avatar dgt41
dgt41 - comment - 24 Apr 2016

@smz JLayouts are supposed to be HTML files so that front end devs can consume easier. I also try to concatenate those vars but I messed up the whole thing

avatar brianteeman brianteeman - change - 25 Apr 2016
Status Pending Ready to Commit
avatar brianteeman
brianteeman - comment - 25 Apr 2016

Setting RTC and a milestone of 3.6

I assume you will now be working on the other fieldtypes


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

avatar joomla-cms-bot joomla-cms-bot - change - 25 Apr 2016
Labels Added: ?
avatar brianteeman brianteeman - change - 25 Apr 2016
Milestone Added:
avatar dgt41
dgt41 - comment - 25 Apr 2016

@brianteeman I will make PR's for all of them this week, but I need some time for calendar and other fields with JS in order to make sure that will be compatible with the new repeatable field. (I've got the code but haven't tested it against @fedik 's PR)

avatar brianteeman
brianteeman - comment - 25 Apr 2016

Great

avatar rdeutz rdeutz - change - 2 May 2016
Labels Added: ?
avatar rdeutz rdeutz - change - 2 May 2016
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2016-05-02 06:37:11
Closed_By rdeutz
avatar rdeutz rdeutz - close - 2 May 2016
avatar rdeutz rdeutz - merge - 2 May 2016
avatar joomla-cms-bot joomla-cms-bot - close - 2 May 2016
avatar joomla-cms-bot joomla-cms-bot - change - 2 May 2016
Labels Removed: ?

Add a Comment

Login with GitHub to post a comment