[#20221] - Fix errors exposed by strict and also improve js
- Fixed in Code Base
- 9 Aug 2018
- Medium
- Build: staging
- # 20221
- Diff
- okonomiyaki3000:fix-errors-exposed-by-strict-and-also-improve-js
User tests: Successful: Unsuccessful:
Pull Request for Issue # .
Summary of Changes
This is just #12544 plus some additional improvements.
While that older PR was aimed only at fixing the "strict" errors, this one fixes other problems.
In particular, button groups (radio buttons) will now work properly when used inside of subforms.
Same for Chosen selectors that use coloring based on state.
Also, delegation is used to handle various events. This has several benefits.
This is only a rewrite of the first part of this file. There is likely much more that can be improved.
Testing Instructions
This file only affects the administrator when using the Isis template so use that.
First of all, nothing should be broken. This mainly affects Chosen, radio button groups, and tooltips. So check any pages that have those things.
The main improvements apply to subforms. Subforms with radio buttons set up as btn-group should be tested. There are probably none of these in Joomla because they didn't really work before this patch. So make one and test it. Same goes for Chosens used in subforms with the chzn-color-state class.
Expected result
When you dynamically add a subform row that contains radio buttons, Chosen, or tooltips, these things should be initialized properly in the new row. Before this patch they were not.
Actual result
Oh, it works.
Documentation Changes Required
Probably not. Unless there were previously some documented warnings about using radio buttons or Chosens in subforms. If so, those warnings may be deleted as they will no longer apply.
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | JavaScript Administration Templates (admin) |
This 'drone' failure seems to have nothing at all to do with these changes.
@Quy This PR addresses the Isis template only, #19722 addresses beez3 and protostar as well. Since this one is on top of #12544, it also includes the "strict" fixes which really should have been merged by now. Other than that, it's really an issue of which adds more overhead and complexity to the code and which one reduces it. I'll recuse myself from sharing any opinion on that matter.
Just few comments here:
- First of all thanks for this PR
- The js functionality for the group buttons does not belong to the template, it must be triggered in the layout of the checkboxes (or whatever the layout name is that renders those buttons). These are joomla fields and the js should be there!
- Unfortunately this cannot be done in J3 without risking breaking a lot of components/plugins/modules
- For J4 these buttons (as all the rest of the fields) will be done using custom elements (one of the good things with this approach is that we don't need and coupling [events or callback] for the subform)
- It would be greatly appreciated if you can do a PR against 4.0-dev to remove the jQuery dependency and make this a custom element and also couple it to the actual layout. That is already in our todo list: #19427
LOL, wait.... This is going to take more than most of these. I better jump in over there but please don't think of it as related to this PR and don't expect the fix very soon.
What are we gonna do about this drone failure? Is this for real? I don't buy it.
| Labels |
Added:
?
|
||
Just make it run again. I bet it works this time.
Drone restarted
I don't know why this would fail. This PR has nothing to do with jquery.ui.sortable or whatever.
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/20221.
@okonomiyaki3000 I only noticed this PR after merging #12544 and I think because of that merge we have a conflict now. Could you update this PR please?
Rebased with latest staging. Should be correct.
Don't believe appveyor, it just timed out.
I have tested this item
Subforms are working with this patch, even when I click on the plus.
For testing I added this to excludes.xml:
<field name="myradiovalue"
type="radio"
default="0"
label="Select an option"
class="btn-group">
<option value="0">1</option>
<option value="1">2</option>
</field>
<field name="myselectvalue"
type="list"
default="0"
label="Select an option"
class="chzn-color-state">
<option value="0">nimm</option>
<option value="1">mich</option>
<option value="2">nicht</option>
</field>
```<hr /><sub>This comment was created with the <a href="https://github.com/joomla/jissues">J!Tracker Application</a> at <a href="https://issues.joomla.org/tracker/joomla-cms/20221">issues.joomla.org/tracker/joomla-cms/20221</a>.</sub>
@viocassel can you please retest?
I have tested this item
It works
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/20221.
| Status | Pending | ⇒ | Ready to Commit |
RTC
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/20221.
mbabker
- | Status | Ready to Commit | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2018-08-09 12:49:51 |
| Closed_By | ⇒ | mbabker | |
| Labels |
Added:
?
|
||
If you want to test subforms, there's one here:
plugins/system/redirect/form/excludes.xmlThis creates a repeatable subform in the Redirect plugin config. If you add some fields this file, you can easily test that they are working properly.