User tests: Successful: Unsuccessful:
There is a condition where the multilingual status module and patch tester conflict for reasons I've not been able to debug (see joomla-extensions/patchtester#92 for more info), but the solution for this issue seems to be in the status module.
JHtmlBootstrap::modal is documented as broken since 3.4.0 yet is still used in the module. Removing the call to the broken method seems to fix the conflict I'm hitting without breaking the module.
Uncaught TypeError: Cannot read property 'options' of null
message The component's "Fetch Data" modal nor the modal for the module will work correctly either.Codestyle fixes in the module's layout and correctly calling the JHtmlBootstrap::renderModal() method through the JHtml::_ loader.
Yes it should be called. An extension, core or otherwise, should not depend on something else loading its dependencies.
So maybe we should as well add JHtml::_('bootstrap.framework');
since we also depend on that!
Never mind that I see that BS loads itself!
I'd say it's a separate PR, I'd also say you could actually totally remove that call as renderModal calls bootstrap.framework which calls jquery.framework.
JHtmlBootstrap::renderModal(
vs JHtml::_('bootstrap.renderModal'
The first one on my IDE with cmd + click on JHtmlBootstrap
takes me to the correct file/class but the other takes me to JHtml and cmd + click on 'bootstrap.renderModal'
isn’t really helpful as well. So I am a little bit confused here...
JHtml has some code that can let you override the various methods if called via JHtml::_. Direct calling of JHtml (and its subclasses) methods break that functionality. The downside is just as you pointed out, IDE autocompletion is out the window.
IMHO JHtmlBootstrap::Modal()
should be patched and just return;
: its code is broken and useless.
For the reasons why it is still there, please see: #5087 (comment)
If someone can fix the broken method then I'd say do that, otherwise you're right that it should just return if it's fatally broken.
Still a separate PR. We have a bad habit of breaking single responsibility principles in our code and our patches (and I do realize I'm now the pot calling the kettle black considering what I changed in the layout file here).
AFAIK, it was meant as a support method for the "old" JHtmlBootstrap::renderModal()
but it is unneed in that role right now.
The rationale for keeping it as it was is that somewone could have used it for other reasons and removing or modifying it could have been a breach of B/C. Personally I don't worship to the B/C god that far, but I think that removing it is a decision that should be taken by the PLT.
If it's fatally broken then I'd say dump the code and just make it return, leave a comment explaining why. As for why you can't just remove the method completely, it's a publicly callable API method. Leaving it doing nothing doesn't break anyone's code (unless they depend on that broken JS) but removing it entirely results in fatal errors for not found methods. There's no pleasing everyone here, but if we know the method's broken then do something about it other than deprecate it with a vague comment.
I totally agree...
Do you want me to create a PR along such lines?
I hope someone does. I need to put GitHub on ignore and get back to writing talks
Category | ⇒ | Administration Modules |
Status | New | ⇒ | Ready to Commit |
Easy | No | ⇒ | Yes |
@test successful. Both patchtester and the modul still works. Thanks RTC
Labels |
Added:
?
?
|
Labels |
Added:
?
|
Labels |
Added:
?
|
Status | Ready to Commit | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2015-05-11 08:20:04 |
Closed_By | ⇒ | mbabker |
Milestone |
Added: |
Labels |
Removed:
?
|
@test ok!
do we need to call jquery.framework here?
Both isis and hathor load jquery and bootstrap...