Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use the JHtml::_ function instead of calling functions directly. #12546

Merged
merged 4 commits into from Feb 5, 2017

Conversation

okonomiyaki3000
Copy link
Contributor

@okonomiyaki3000 okonomiyaki3000 commented Oct 25, 2016

Pull Request for Issue # .

Summary of Changes

Instead of calling JHtml static functions directly, call them with the underscore function like JHtml::_('functionname'); so that it's possible to override them.

Testing Instructions

The changed files could potentially affect many pages (for example, any list page with a disabled ordering column). You should see no difference before and after applying the patch.

Documentation Changes Required

No but, if there is some documentation for developers, it would be a good idea for that documentation to make it clear that JHtml static functions should not be called directly but invoked through the _ function.

Copy link
Contributor

@mbabker mbabker left a comment

Choose a reason for hiding this comment

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

Please revert the changes in JHtmlTest. We want to explicitly only test the actual functions there, not that the loader is able to resolve to the correct function as well (we have tests for that part specifically already).

@@ -670,7 +670,7 @@ public static function stylesheet($file, $attribs = array(), $relative = false,
*
* @return mixed nothing if $path_only is false, null, path or array of path if specific js browser files were detected.
*
* @see JHtml::stylesheet()
* @see JHtml::_('stylesheet', )
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you revert this line too (and if there are any others in doc blocks, those as well)?

@okonomiyaki3000
Copy link
Contributor Author

@mbabker tests have been reverted, please check.

@ghost
Copy link

ghost commented Jan 12, 2017

I have tested this item ✅ successfully on 3973dfb

Tested on Menu Item Category List and hidden Show Author. Page display same with or without PR-


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

@dgrammatiko
Copy link
Contributor

@okonomiyaki3000 can you update your PR, it's out of sync, Thanks

@okonomiyaki3000
Copy link
Contributor Author

Should be in sync now.

@ThomasFinnern
Copy link

ThomasFinnern commented Feb 5, 2017

I have tested this item ✅ successfully on ae82474

Please check whether the operations performed meet the requirements for this test (newbee on testing)

On new installation PHP7.1.1 and Joomla! 3.7.0-beta2 dev [ Amani ] 2-February-2017 18:53 GMT:

@zero-24
Copy link
Contributor

zero-24 commented Feb 5, 2017

I have just fixed the conflicts. -> RTC

@zero-24 zero-24 added this to the Joomla 3.7.0 milestone Feb 5, 2017
@zero-24 zero-24 added the RTC This Pull Request is Ready To Commit label Feb 5, 2017
@wilsonge wilsonge merged commit c5dfb6c into joomla:staging Feb 5, 2017
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Feb 5, 2017
@okonomiyaki3000 okonomiyaki3000 deleted the use-jhtml-underscore branch February 5, 2017 23:54
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.

None yet

7 participants