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

[1.3][1.4] JCSSUtil.php pageaddvar prototype/jquery order and file naming #2170

Closed
espaan opened this issue Jan 8, 2015 · 7 comments
Closed

Comments

@espaan
Copy link
Contributor

espaan commented Jan 8, 2015

Hi,
I'm testing out a new theme conversion with Bootstrap and jQuery.

  1. jquery / prototype ordering
    When I'm using
    {pageaddvar name='javascript' value='prototype'}
    {pageaddvar name='javascript' value='jquery'}

with the current JCSSUtil.php (1.3 for sure and 1.4 afais) the order is always jquery first, then noconflict.js or jquery_config.js in 1.4 and only then prototype. For the noconflict to work properly afaik the prototype should be loaded first.
So

<script type="text/javascript" src="javascript/jquery/jquery.min.js"></script>
<script type="text/javascript" src="javascript/jquery/noconflict.js"></script>
<script type="text/javascript" src="javascript/ajax/proto_scriptaculous.combined.min.js"></script>
<script type="text/javascript" src="javascript/helpers/Zikula.js"></script>

with a small change in the JCSSUtil.php by putting prototype above in the $scripts array
https://github.com/zikula/core/blob/1.4/src/lib/util/JCSSUtil.php#L365
and
https://github.com/zikula/core/blob/1.3/src/lib/util/JCSSUtil.php#L336
that one will always be loaded before jquery and noconflict and the order will be

<script type="text/javascript" src="javascript/ajax/proto_scriptaculous.combined.min.js"></script>
<script type="text/javascript" src="javascript/jquery/jquery.min.js"></script>
<script type="text/javascript" src="javascript/jquery/noconflict.js"></script>
<script type="text/javascript" src="javascript/helpers/Zikula.js"></script>
  1. follow in [1.3.x] Add jQuery 1.11.2 with migrate as option in 1.3.x  #2223 (Only 1.3.x relevant) Furthermore it would be really convenient to rename the jQuery file to a filename without version number. In that way people using 1.3.x can also easily update to a later version of jQuery just by replacing this file with a newer one. And then no code fiddling is needed. In the code example above I already this in my install BTW. So you don't see the version number any more. In 1.4.x jQuery is loaded via composer and does not have a versionnr.

  2. follow in [1.3.x] Add jQuery 1.11.2 with migrate as option in 1.3.x  #2223 (only 1.3.x relevant) it would be nice to add a jquery-migrate to the list of pageaddvar options for the reason mentioned under 2. When people use a jquery above 1.9.x then they can also load jq-migrate via pageaddvar and it works. In 1.4.x this is already the case, since jQ > 1.9 is used.

I will make a PR later on for this.

@craigh
Copy link
Member

craigh commented Jan 8, 2015

refs #1324 (currently scheduled for 2.0.0)

@Guite
Copy link
Member

Guite commented Jan 8, 2015

@espaan how about moving 2) and 3) into another issue to keep things separated?

@espaan
Copy link
Contributor Author

espaan commented Jan 8, 2015

A good idea. I did it this way since I wanted to make 1 pr for these points :-)

Op 8 jan. 2015 om 19:52 heeft Axel Guckelsberger notifications@github.com het volgende geschreven:

@espaan how about moving 2) and 3) into another issue to keep things separated?


Reply to this email directly or view it on GitHub.

@craigh
Copy link
Member

craigh commented Jan 8, 2015

For the noconflict to work properly afaik the prototype should be loaded first.

is this documented somewhere?

@craigh
Copy link
Member

craigh commented Jan 8, 2015

Furthermore it would be really convenient to rename the jQuery file to a filename without version number.

this may be difficult since we are loading the library from composer/component

@espaan
Copy link
Contributor Author

espaan commented Jan 10, 2015

@craigh I'm reading that the order is always first prototype and then jQ and then noconflict. But not that i is absolutely necessary.
http://api.jquery.com/jquery.noconflict/ says to first load the other lib then jqyery and then no conflict.
also http://learn.jquery.com/using-jquery-core/avoid-conflicts-other-libraries/

The loading from composer is indeed true for 1.4.x, but in 1.3.x jQuery is just within Zikula. For 1.4.x it is also not a problem, since version nr is not in the name already.

  1. also only holds for 1.3.x, since in 1.4.x it is already there.

@Guite Guite modified the milestones: 1.4.0, 1.4.1 Jan 31, 2015
@espaan
Copy link
Contributor Author

espaan commented Feb 3, 2015

Point 2) and 3) can be followed up in #2223
But there will be one PR (per Core version of course) with the change :-)

@craigh craigh closed this as completed Feb 4, 2015
@craigh craigh modified the milestones: 1.4.0, 1.4.1 Feb 4, 2015
espaan added a commit to espaan/core that referenced this issue Feb 6, 2015
| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | -
| Fixed tickets | zikula#2174, zikula#1644
| 1.4 PR        | in progress
| Refs tickets  | zikula#2170
| License       | LGPLv3+
| Doc PR        | -

- Imagine vendor plugin is updated to 0.6.2 (nov2014) from 0.3.0 (2012)
- added width/height autoscale as option in thumbnail generation
- added jpeg_quality/png_compression_level to preset options
- added modulename setting to preset option
- added option to force remove all thumbnails next to thumb cleanup
- updated README and thumb plugin descriptions
- updated CHANGELOG
espaan added a commit to espaan/core that referenced this issue Feb 6, 2015
[1.3.x] Updated Imagine plugin with enhancements

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | -
| Fixed tickets | zikula#2174, zikula#1644
| 1.4 PR        | in progress
| Refs tickets  | zikula#2170
| License       | LGPLv3+
| Doc PR        | -

- Imagine vendor plugin is updated to 0.6.2 (nov2014) from 0.3.0 (2012)
- added width/height autoscale as option in thumbnail generation
- added jpeg_quality/png_compression_level to preset options
- added modulename setting to preset option
- added option to force remove all thumbnails next to thumb cleanup
- updated README and thumb plugin descriptions
- updated CHANGELOG
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants