User tests: Successful: Unsuccessful:
Pull Request for Issue #11299.
Moves some code into a global function that is trigerable by subform-repeatable.js.
Create a form with a repeatable subform containing a radio control with class="btn-group".
Click to add a new row.
Button group on new row would nbot be styled.
Button group is styled.
Status | New | ⇒ | Pending |
Category | ⇒ | JavaScript Administration Templates (admin) Front End Templates (site) |
Title |
|
Labels |
Added:
?
|
Please don't take me wrong, i support this PR,
i hope it is accepted
just this should be moved (aka added by the extension that needs it)
and method should be checked if it exists
$(document).on('subform-row-add', function(event, row) { Joomla.TemplateJs(row); });
the template code should not be aware of sub-form field or any other code
this is a really bad hard coding
yes, and no
for Joomla! 3 it only one possible solution
for Joomla! 4 we have an evens which allow to do this without "hardcoding" see #18864 #16016
For me this pull request is good as it is.
Do not need to move anything, each extension should care about their script on its own, we just provide an event that allow to do it.
@ggppdk Using events is NOT "hard coding" - it is loose coupling. So instead of subform-repeatable having to know exactly what other components needs to know about a row being added and calling them explicitly, instead it just tells the world that it has added a row and lets every other piece of code that is interested listen for the event and handle it.
This is (IMO) a much better way of constructing large complex pieces of code.
Using events is NOT "hard coding" - it is loose coupling. So instead of subform-repeatable having to know exactly what other components needs to know about a row being added and calling them explicitly
i realy don't understand you now, i do not understand why you said the above
i said nothing about the above, nor did i imply it
i will repeat what i said (please read my detailed answer above), the hard-coding i spoke about is
the above is a really silly hard-coding
the only argument that i could understand about doing it is what Fedik said (implied ?) that in J4 there is a more proper solution,
@Sophist-UK basically it's knowing the difference between a template and extension. As stated before, this is a fair fix, however we should not be adding code to the template Javascript file to cater for an extension. The template dictates the design of a site, nothing else. In other words, is there is some Javascript that is needed to fix the subform fields, it should reside in the subform fields Javascript files.
@C-Lodder That makes no sense, particularly for this example.
The subform-repeatable (or indeed 3rd party extensions) cannot know for example that a template uses Chosen to enhance dropdowns. That is a template decision, and subform-repeatable should not need to know about it. But the template needs to recognise that on modern web sites not all HTML arrives at page load time - some comes from Ajax calls - so if the template uses e.g. Chosen, then it needs to provide a way to have it reapplied to the dropdowns by subform-repeatable or any extension that dynamically updates the HTML.
However I agree that subform.row.add
is the wrong approach because it is too specific and the template is tied to that functionality, and joomla:updated
is better because it is more generalised and can be dispatched by any extension that needs to reapply template styling. (I would personally have used something like joomla:htmlUpdated
or joomla:applyTemplateStyle
or something a bit more descriptive, but the concept still holds true.)
The subform-repeatable (or indeed 3rd party extensions) cannot know for example that a template uses Chosen to enhance dropdowns. ...
they do not need to know, they do not need to care about it,
they would only need to know if there is a way to apply template styles
So they could just call Joomla.TemplateJs(container)
(if it exists aka implemented by template) whenever they decide they need to
by registering their own custom events (whatever their names are) that make use of the method
the original arguement (at issue #11299) was
so this discussion is not about what is proper,
we know that proper thing , is to avoid hard-coding for something that is
aka then the above 2 things would justify the trade-off of doing this
Finally since this PR adds the "non-standarize" method of 'Joomla.TemplateJs' , i still fail to see the need for this hard-coding
Anyway this hard-coding will not effect 3rd party extensions, so lets just test and merge it
- you want to see tight coupling by direct calls
no no i did not say not to use events
nor anything related to "tight coupling" thing
Adding Joomla.TemplateJs
to the Joomla
namespace is a big mistake. This function will never need to be called outside of this specific context but, once it has been created there, removing it in the future will be considered a BC break because (in theory) some other extension could use it (even though nobody ever will).
Also, jQuery's ready function should simply be written as jQuery(function($) { });
, this jQuery(document).ready(function () {});
style is really unnecessary.
The purpose of adding Joomla.TemplateJs to the Joomla namespace has been done deliberately because I believe other extensions needs this functionality.
For example when you use a template that uses Chosen dropdowns, and an extension dynamically loads HTML, then it needs to initialise Chosen on the new dropdowns.
This is exactly the purpose of this PR - to make a standard way for extensions to initialise code to match template styling.
@Sophist-UK I see what you're trying to do but I don't think the template is the right place to do it. Not every template will have this function so it's not like any extension that loads html can just call it and expect everything to be fine. On top of that, this function doesn't even do everything that may need to be done. It handles chosen, tooltips, and button groups. There are any number of items that may require initialization that this doesn't cover.
The subform-row-add
event was added to so that anything that needs initialization can do it automatically. The only problem with it is that it is tied to subform. We should have called it 'html-dom-update' or something like that. Then anything that updates the dom can trigger it and anything that needs initialization can listen for it.
Plus, I'm sorry to say, you're adding a bit of complexity to these javascript files when they are already more complex than they need to be. They can actually be simplified and accomplish the same thing.
@okonomiyaki3000 - Lots to discuss here...
Not every template will have this function so it's not like any extension that loads html can just call it and expect everything to be fine.
True - so if we use this approach then I guess there needs to be a default function that is overridden by a template - and that sounds messy. So it is probably better to use an event listener instead.
The
subform-row-add
event was added to so that anything that needs initialization can do it automatically. The only problem with it is that it is tied to subform. We should have called ithtml-dom-update
or something like that. Then anything that updates the dom can trigger it and anything that needs initialization can listen for it.
Actually I think that there are two separate events needed here:
html-dom-updated
as you suggest.,<div>
containing a counter of the number of rows.So perhaps we should issue a second html-dom-updated
event immediately before the subform-row-add
event. I don't think this will introduce any incompatibility because no one currently uses the new event and they can switch at their leisure.
On top of that, this function doesn't even do everything that may need to be done. It handles chosen, tooltips, and button groups. There are any number of items that may require initialization that this doesn't cover.
All I am doing is adding what I know needs adding. If there are others then either point them out to me and I will do them, or others can add them when they find they are missing.
Plus, I'm sorry to say, you're adding a bit of complexity to these javascript files when they are already more complex than they need to be. They can actually be simplified and accomplish the same thing.
Happy to look at simplifying them once we have agreed the above.
P.S. On consideration I now agree that Joomla.TemplateJs
was indeed "a big mistake". Thanks for being patient with me whilst I came to realise this. :-)
The subform-row-add event was added to so that anything that needs initialization can do it automatically. The only problem with it is that it is tied to subform. We should have called it 'html-dom-update' or something like that. Then anything that updates the dom can trigger it and anything that needs initialization can listen for it.
@okonomiyaki3000 it already done for Joomla 4, see #16016 and #18864
So perhaps we should issue a second
html-dom-updated
event immediately before thesubform-row-add
event. I don't think this will introduce any incompatibility because no one currently uses the new event and they can switch at their leisure.
This is not bad. The html-dom-updated
event object could also contain some kind of information about exactly what type of thing was being updated but then, how would such a thing even be identified? So maybe having two separate events is the best way. How about if you have two subforms on the same page? Do we need a way to differentiate them so that we know which one the events came from? I guess this is already not hard to find out if we really care to know.
All I am doing is adding what I know needs adding. If there are others then either point them out to me and I will do them, or others can add them when they find they are missing.
That's just it: there is not a finite list of things. It can depend on any number of factors. A centralized solution can't really work.
Closed in favour of PRs #20221 / #20222 / #20224 / #20225.
@okonomiyaki3000 Actually there are two finite lists of things. The template needs to hook into the html-dom-updated
event to implement the finite list of things it uses, and then extensions also need to hook into both events to implement the finite list of things they know about.
Status | Pending | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2018-04-25 07:33:08 |
Closed_By | ⇒ | Sophist-UK |
@Sophist-UK Yes, the template can support a finite list of things and each individual extension can support a finite list of things. What I'm saying is that the list of all possible things that may need to be supported is not finite.
@okonomiyaki3000 If we are going to be literal about it, the list is definitely finite just unknown.
My point is that the different authors of the various templates and extensions know what their own code needs, so we need event infrastructure that enables them to easily write it and hook in.
@Sophist-UK @okonomiyaki3000 just a note here:
as @Fedik already mentioned in J4 we have proper event system for the subform but even better we're implementing all joomla fields as Custom Elements and thus we are actually utilising the native browser (DOM) events for the subforms. Since custom elements are ES6 classes that have a constructor and lifecycle events like connectedCallback
and disconnectedCallback
we don't need to fire our own Joomla-whatever
or as @Sophist-UK mentioned the html-dom-updated
events. We use the basic connectedCallback
which is essentially the same without writing any extra lines of code! WIN
@dgrammatiko Awesome. I always thought that would be the best way. I was also thinking about this:
https://stackoverflow.com/questions/3219758/detect-changes-in-the-dom#14570614
Which it seems we will not need in J4 and which might not be suitable for J3... Doesn't work below IE9. Is J3 expected to support older browsers?
@okonomiyaki3000 there is a promise that J3 should be IE8 compatible. But mutation observer is polyfillable for IE8 and we already have a JHtml method to add polyfills (eg: https://github.com/megawac/MutationObserver.js/tree/master)
You have done exactly what i was talking at #11299, except a very bad thing (see below)
Joomla.applyTemplateStyles
you called itJoomla.TemplateJs
but there is a very bad "hard coding" in this PR
i mean what if the sub-form field later needs the JS-based styles being applied for a different event too, are you going to update the template(s) again and its overrides too ?
can you also add 13 more hardcoding for 13 other extensions that need to call template styles when they want to add some contents into the DOM ? (i have a few)
At least if my suggestion of standarizing the method was accepted then
the template code should not be aware of sub-form field or any other code
this is a really bad hard coding