Skip to content

Comments

[4.0] WebAsset for JHtmlJquery, JHtmlSearchtools#23112

Merged
wilsonge merged 10 commits intojoomla:4.0-devfrom
Fedik:asset-2
Dec 9, 2018
Merged

[4.0] WebAsset for JHtmlJquery, JHtmlSearchtools#23112
wilsonge merged 10 commits intojoomla:4.0-devfrom
Fedik:asset-2

Conversation

@Fedik
Copy link
Member

@Fedik Fedik commented Nov 18, 2018

Summary of Changes

The patch add use of WebAsset for JHtmlJquery, JHtmlSearchtools scripts.

Testing Instructions

The site should work as before, without errors and visible changes.

For reference #22435

Copy link
Member

@laoneo laoneo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use the document from the application as Factory::getDocument is deprecated.

laoneo and others added 6 commits November 20, 2018 09:30
Co-Authored-By: Fedik <getthesite@gmail.com>
Co-Authored-By: Fedik <getthesite@gmail.com>
Co-Authored-By: Fedik <getthesite@gmail.com>
Co-Authored-By: Fedik <getthesite@gmail.com>
Co-Authored-By: Fedik <getthesite@gmail.com>
@Fedik
Copy link
Member Author

Fedik commented Nov 20, 2018

@laoneo updated, thanks!

@mbabker
Copy link
Contributor

mbabker commented Nov 20, 2018

We shouldn't be using code snippets like this for deprecation statements. Remember these render on docs pages like the API site. So a @deprecated 5.0 Use $app->getDocument()->getWebAssetManager()->enableAsset('jquery.ui.core'); tag has absolutely no context, WTF is $app? A magic global? A stdClass object?

If you're suggesting an alternative it should always be in the format of Use <class>::<method> instead. Use the documentation for full code examples of how to use an API.

@Fedik
Copy link
Member Author

Fedik commented Nov 20, 2018

@mbabker then Factory::getApplication()->.... is fine, or?

@mbabker
Copy link
Contributor

mbabker commented Nov 20, 2018

You know what, just use Factory::getApplication()->whatever.

The right way to do it would be to say Use Joomla\CMS\WebAsset\WebAssetRegistry::enableAsset() and leave it at that (because it's not a deprecation tag's or log message's responsibility to give anyone an exact copy/paste snippet on what the replacement is, the tag and notice are there to tell people what the replacement is and the core documentation would be the resource developers go to to learn how to actually use a feature/API, but we all know the docs are purely end user facing resources and the actual code has nothing but the automatically generated references from the doc blocks). But people seem to think the doc blocks have to be self documenting on how to actually use the code, so whatever.

@wilsonge wilsonge merged commit 51e47b0 into joomla:4.0-dev Dec 9, 2018
@wilsonge
Copy link
Contributor

wilsonge commented Dec 9, 2018

Sorry for the delay here. And thanks :)

@wilsonge wilsonge added this to the Joomla 4.0 milestone Dec 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants