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

JHtml and JDocument script/stylesheet/image improvements #24

Open
wants to merge 54 commits into
base: staging
Choose a base branch
from

Conversation

andrepereiradasilva
Copy link
Owner

Summary of Changes

This PR does some changes to JHtml image, stylesheet and script methods:

  • Simplifies methods signatures for B/C and F/C. This will allow to add attribs to JHtml script and maybe more (since the signature is not fixed now) in a simpler way in the future.
  • Adds versioning option to stylesheet, script and image methods.
  • Deprecates MD5SUM version scheme
  • Allows version in urls already with &
  • Allows to add inline JS and CSS with inline = true
  • Adds more options to generate mediaversion (by Fedik)
  • Ability to add other attributes to the script tag (merges PR 8540)
  • Allows to add relative path in template (ex: https://github.com/andrepereiradasilva/joomla-cms/blob/jhtml-versio/administrator/templates/isis/index.php#L32)

After this PR you can also scripts like this (the same is valid for stylesheet and image methods):

Scripts
/* 
 * Script default values:
 * - Load framework [framework] = No
 * - Relative paths [relative] = No
 * - Return only path [path_only] = No
 * - Try to load browser specific files [detect_browser] = Yes
 * - Try to load only debug script (not compressed) if debug mode [detect_debug] = Yes
 * - Media version [version] = none [NEW]
 * - Attributes [attribs] = none [NEW]
 *
 * Code examples: 
 * - 1: Load script disabling browser detect (faster).
 * - 2: Load script with framework.
 * - 3: Load script with relative path.
 * - 4: Load script with auto version.
 * - 5: Load script with custom version.
 * - 6: Load script with custom data attributes and id.
 */

/* Current method */
JHtml::_('script', 'media/jui/js/chosen.jquery.js', false, false, false, false, true);
JHtml::_('script', 'media/jui/js/fielduser.js', true);
JHtml::_('script', 'system/passwordstrength.js', false, true);
// Not supported
// Not supported
// Not supported

/* New method */
JHtml::_('script', 'media/jui/js/jquery.minicolors.js', array('detect_browser' => false));
JHtml::_('script', 'media/jui/js/jquery.searchtools.js', array('framework' => true));
JHtml::_('script', 'system/helpsite.js', array('relative' => true));
JHtml::_('script', 'media/jui/js/jquery.ui.core.js', array('version' => 'auto'));
JHtml::_('script', 'media/jui/js/jquery.ui.sortable.js', array('version' => 'v3.5.0'));
JHtml::_('script', 'media/jui/js/sortablelist.js', array('attribs' => array('id' => 'myid', 'data-value' => 'my-values')));
Styles
/* 
 * Stylesheet default values:
 * - Attributes [attribs] = none
 * - Relative paths [relative] = No
 * - Return only path [path_only] = No
 * - Try to load browser specific files [detect_browser] = Yes
 * - Try to load only debug style (not compressed) if debug mode [detect_debug] = Yes
 * - Media version [version] = none [NEW]
 *
 * Code examples: 
 * - 1: Load stylesheet disabling browser detect (faster).
 * - 2: Load stylesheet with relative path.
 * - 3: Load stylesheet with auto version.
 * - 4: Load stylesheet with custom version.
 * - 5: Load stylesheet with custom data attributes and id.
 */

/* Current method */
JHtml::_('stylesheet', 'media/jui/css/jquery.minicolors.css', array(), false, false, false, true);
JHtml::_('stylesheet', 'system/frontediting.css', array(), true);
// Not supported
// Not supported
JHtml::_('stylesheet', 'media/jui/css/jquery.searchtools.css', array('id' => 'myid', 'data-value' => 'my-value'));

/* New method */
JHtml::_('stylesheet', 'media/jui/css/jquery.minicolors.css', array('detect_browser' => false));
JHtml::_('stylesheet', 'system/system.css', array('relative' => true));
JHtml::_('stylesheet', 'media/jui/css/sortablelist.css', array('version' => 'auto'));
JHtml::_('stylesheet', 'media/jui/css/jquery.simplecolors.css', array('version' => 'v3.5.0'));
JHtml::_('stylesheet', 'media/jui/css/jquery.searchtools.css', array('attribs' => array('id' => 'myid', 'data-value' => 'my-value')));
Images
/* 
 * Image default values:
 * - Alternative text [alt] = none
 * - Attributes [attribs] = none
 * - Relative paths [relative] = No
 * - Return html tag without (-1) or with file computing(No). Return computed path only (Yes) [path_rel] = No
 * - Media version [version] = none [NEW]
 *
 * Code examples: 
 * - 1: Load image without alternative text.
 * - 2: Load image with relative path.
 * - 3: Load image with auto version.
 * - 4: Load image with custom version.
 * - 5: Load image with custom data attributes and id.
 */

/* Current method */
JHtml::_('image', 'media/mod_languages/images/icon-16-language.png', '');
JHtml::_('image', 'mod_languages/icon-16-language.png', '', true);
// Not supported
// Not supported
JHtml::_('image', 'media/mod_languages/images/icon-16-language.png', '', array('id' => 'myid', 'data-value' => 'my-value'));

/* New method */
JHtml::_('image', 'media/mod_languages/images/icon-16-language.png');
JHtml::_('image', 'mod_languages/icon-16-language.png', array('relative' => true));
JHtml::_('image', 'media/mod_languages/images/icon-16-language.png', array('version' => 'auto'));
JHtml::_('image', 'media/mod_languages/images/icon-16-language.png', array('version' => 'v3.5.0'));
JHtml::_('image', 'media/mod_languages/images/icon-16-language.png', array('attribs' => array('id' => 'myid', 'data-value' => 'my-value')));

@andrepereiradasilva
Copy link
Owner Author

@mbabker how about now. What do you think?
If you have comments or suggestions for improvements please say so. I really would appreciate your help on this.

I tested this PR in the patchtester and all seems fine (php 7).

Note: i did this only on my repo. will not push to joomla repo until is fine.

@andrepereiradasilva andrepereiradasilva changed the title add version to jhtml JHtml script/stylesheet/image improvements Apr 3, 2016
*
* @return array files to be included.
*
* @see JBrowser
* @since 1.6
*
* @deprecated 4.0 Usage of MD5SUM files is deprecated, use version instead.
Copy link

Choose a reason for hiding this comment

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

This method isn't deprecated, don't add a tag here.

*
* @return mixed nothing if $options[path_only] is false, null, path or array of path if specific css browser files were detected.
*
* @since 3.6
Copy link

Choose a reason for hiding this comment

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

Leave the since tag alone, this indicates the first version the method was available in the API, not the version it was last changed.

@andrepereiradasilva
Copy link
Owner Author

ok i'm making all the changes you said. Thanks a lot.

I would like more if the methods were called like (example):

JHtml::_('script', 'system/somejs.js', array('version' => null);
JHtml::_('stylesheet', 'system/somestyle.css', array('version' => null);
JHtml::_('image', 'system/someimage.png', array('version' => null);

In other words the $fileremaining in the method signature.
But i didn't find a way since the stylesheet already as an array in the second argument. If you have an hint how to solve this i would go that way.

Also do you think a similar signature revision should be made for the four addXxxx(Version) methods in JDocument?

@mbabker
Copy link

mbabker commented Apr 3, 2016

For stylesheet it'd only work if there were unique (required) keys in both the attribs and new signature's arrays. Since file's the only requirement if you leave that as a separate parameter it's hard to find something to go from.

I'd leave JDocument alone for the moment and just deal with JHtml.

@andrepereiradasilva
Copy link
Owner Author

so i can conclude that in your opinion, the only way is to leave as i made in this PR. the other opinion is having different signature for style which i don't want to, because it would be confusing.

@andrepereiradasilva
Copy link
Owner Author

ok, found a way
https://github.com/andrepereiradasilva/joomla-cms/blob/jhtml-versio/libraries/cms/html/html.php#L614-L626
Now i can have the more friendly signatures!

JHtml::_('script', 'system/somejs.js', array('version' => 'auto');
JHtml::_('stylesheet', 'system/somestyle.css', array('version' => 'auto');
JHtml::_('image', 'system/someimage.png', array('version' => 'auto');

@andrepereiradasilva andrepereiradasilva changed the title JHtml script/stylesheet/image improvements JHtml and JDocument script/stylesheet/image improvements Apr 11, 2016
andrepereiradasilva pushed a commit that referenced this pull request Apr 12, 2016
Correct my syntax error from before
andrepereiradasilva pushed a commit that referenced this pull request Apr 21, 2017
* alpha order com_content

* Correcting alpha order (#24)
andrepereiradasilva pushed a commit that referenced this pull request Jun 3, 2017
* codestyle

* code style

* codestyle

* codestyle

* codestyle

* thanks @wojsmol

* corrections - thanks @Quy

* corrections - thanks @Quy

* oops

* make @Quy happy

* Update article.xml

* Remove space

* Update config.xml (#14)

* Update filter.xml (#15)

* Update config.xml (#16)

* Update profile.xml (#17)

* Update application.xml (#18)

* Update article.xml (#19)

* Update filter_articles.xml (#20)

* Update config.xml (#24)

* Update config.xml (#23)

* Update filter_fields.xml (#22)

* Update filter_featured.xml (#21)

* Update override.xml (#25)

* Update config.xml

* Update config.xml (#26)

* Update itemadmin_alias.xml (#30)

* Update itemadmin.xml (#29)

* Update item.xml (#27)

* Update item_alias.xml (#28)

* Update itemadmin_url.xml (#31)

* Update module.xml (#32)

* Update plugin.xml (#33)

* Update config.xml (#34)

* Update link.xml (#35)

* Update config.xml (#36)

* Update style.xml (#38)

* Update config.xml (#37)

* Update note.xml (#42)

* Update group.xml (#41)

* Update filter_debuggroup.xml (#40)

* Update config.xml (#39)

* corrections for @andrepereiradasilva

* gotya
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.

2 participants