PR-staging RTC

Pending

User tests: Successful: Unsuccessful:

avatar brianteeman
brianteeman
4 Oct 2017

Create a color field for content and set it to be required
Open an article to edit and view the source
You will see
<input type="text" name="jform[com_fields][colour]" id="jform_com_fields_colour" value="none" placeholder="#rrggbb" class="minicolors required" data-position="default" data-control="hue"1 data-format="hex" data-validate="color"/>

Note thedata-control="hue"1 the 1 should not be there

This PR resolves that

PR for #18067

avatar joomla-cms-bot joomla-cms-bot - change - 4 Oct 2017
Category Layout
avatar brianteeman brianteeman - open - 4 Oct 2017
avatar brianteeman brianteeman - change - 4 Oct 2017
Status New Pending
avatar Quy Quy - test_item - 4 Oct 2017 - Tested successfully
avatar Quy
Quy - comment - 4 Oct 2017

I have tested this item successfully on 93ef58c


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

avatar franz-wohlkoenig franz-wohlkoenig - test_item - 5 Oct 2017 - Tested successfully
avatar franz-wohlkoenig
franz-wohlkoenig - comment - 5 Oct 2017

I have tested this item successfully on 93ef58c


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

avatar franz-wohlkoenig franz-wohlkoenig - change - 5 Oct 2017
Status Pending Ready to Commit
Easy No Yes
avatar franz-wohlkoenig
franz-wohlkoenig - comment - 5 Oct 2017

RTC after two successful tests.

avatar mbabker
mbabker - comment - 5 Oct 2017

Err, something doesn't seem right here. This PR removes the ability to set a color field as readonly or required.

avatar brianteeman
brianteeman - comment - 5 Oct 2017

I dont think readonly was there before - but required does indeed appear to be broken - please remove rtc and I will look at it later

avatar mbabker mbabker - change - 5 Oct 2017
Status Ready to Commit Pending
avatar brianteeman
brianteeman - comment - 5 Oct 2017

@mbabker I just double checked and required does work correctly with this PR

screenshotr13-42-11

screenshotr13-44-43

avatar mbabker
mbabker - comment - 5 Oct 2017

Then something's not right, you're removing $readonly and $required variables which tells me that neither of those attributes should be in the resulting output unless they're being attached to something else, which seems wrong too. Either way I can look more later too, about to leave for work.

avatar brianteeman
brianteeman - comment - 5 Oct 2017

Thanks - tbh the code in the plugin is a bit of a mess and there is JS involved as well

avatar franz-wohlkoenig franz-wohlkoenig - change - 27 Oct 2017
Status Pending Needs Review
avatar franz-wohlkoenig franz-wohlkoenig - change - 27 Nov 2017
Category Layout com_fields Layout
avatar franz-wohlkoenig franz-wohlkoenig - change - 27 Nov 2017
Title
[com_fields]Color plugin - invalid markup
[com_fields] Color plugin - invalid markup
avatar joomla-cms-bot joomla-cms-bot - edited - 27 Nov 2017
avatar joomla-cms-bot joomla-cms-bot - change - 27 Nov 2017
Category Layout com_fields Layout
avatar Quy
Quy - comment - 1 Dec 2017

There isn't a readonly setting for custom color field.
validate.js dynamically adds the aria-required and required attributes so it is safe to remove $required.

avatar franz-wohlkoenig
franz-wohlkoenig - comment - 10 Dec 2017

@brianteeman can you please look at comment of @Quy above?

avatar brianteeman
brianteeman - comment - 11 Dec 2017

I have double checked
The comment from @Quy is correct and is a reply to @mbabker

$readonly is removed because there is no option in the color field to make it readonly
$required is removed because it is added by JS and thats where the erroneous 1 was coming from

avatar Quy
Quy - comment - 11 Dec 2017

@brianteeman Let's revert $readonly since $required is the main issue.

avatar brianteeman brianteeman - change - 11 Dec 2017
Labels Added: PR-staging
avatar Quy Quy - test_item - 11 Dec 2017 - Tested successfully
avatar Quy
Quy - comment - 11 Dec 2017

I have tested this item successfully on 059f351


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

avatar franz-wohlkoenig franz-wohlkoenig - test_item - 11 Dec 2017 - Tested successfully
avatar franz-wohlkoenig
franz-wohlkoenig - comment - 11 Dec 2017

I have tested this item successfully on 059f351


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

avatar franz-wohlkoenig franz-wohlkoenig - change - 11 Dec 2017
Status Needs Review Ready to Commit
avatar franz-wohlkoenig
franz-wohlkoenig - comment - 11 Dec 2017

Ready to Commit after two successful tests.

avatar mbabker mbabker - change - 21 Dec 2017
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2017-12-21 00:12:00
Closed_By mbabker
Labels Added: RTC
avatar mbabker mbabker - close - 21 Dec 2017
avatar mbabker mbabker - merge - 21 Dec 2017
avatar brianteeman
brianteeman - comment - 21 Dec 2017

Thanks

Add a Comment

Login with GitHub to post a comment