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

Behavior Modal loading mootools library unnecessarily #7887

Closed
akfaisel opened this issue Sep 15, 2015 · 6 comments
Closed

Behavior Modal loading mootools library unnecessarily #7887

akfaisel opened this issue Sep 15, 2015 · 6 comments

Comments

@akfaisel
Copy link
Contributor

Steps to reproduce the issue

Importing the modal library loads mootools library which should be removed.

JHtml::_('behavior.modal', 'a.modal');

Expected result

Do not load mootools library

Actual result

<script src="/sitename/media/system/js/mootools-core.js" type="text/javascript"></script>
<script src="/sitename/media/system/js/mootools-more.js" type="text/javascript"></script>

System information (as much as possible)

In * /libraries/cms/html/behavior.php_, _public static function modal*, the following line is called which is not necessary at all.

// Include MooTools framework
static::framework(true);

If we dig more down the same function, the script declaration has the syntax of jQuery.

// Attach modal behavior to document
$document
    ->addScriptDeclaration(
    "
jQuery(function($) {
    SqueezeBox.initialize(" . $options . ");
    SqueezeBox.assign($('" . $selector . "').get(), {
        parse: 'rel'
    });
});
function jModalClose() {
    SqueezeBox.close();
}"
);

Additional comments

@zero-24
Copy link
Contributor

zero-24 commented Sep 15, 2015

@dgt41 can you have a look into here? I'm not on the latest state regarding JQuery / Mootools ;)

@dgrammatiko
Copy link
Contributor

But if you remove the mootools dependency then you get errors in your console and modal is not working:
screen shot 2015-09-15 at 3 07 49

So, unfortunately modal.js needs the mootools scripts

@mbabker
Copy link
Contributor

mbabker commented Sep 15, 2015

SqueezeBox is dependent on MooTools to function (see the project site http://digitarald.de/project/squeezebox/) so it is indeed a required dependency to load it.

@mbabker mbabker closed this as completed Sep 15, 2015
@akfaisel
Copy link
Contributor Author

I'm sorry, I have failed to check this without loading mootools. I guess
like modal.js should be upgraded or converted to be used with jQuery as
Joomla has already moved away from mootools.
On 15 Sep 2015 21:16, "Michael Babker" notifications@github.com wrote:

Closed #7887 #7887.


Reply to this email directly or view it on GitHub
#7887 (comment).

@mbabker
Copy link
Contributor

mbabker commented Sep 16, 2015

modal.js is the SqueezeBox modal. IMO that shouldn't be touched unless they have a jQuery compatible version of their code, it's not our place to be hacking third party code.

@dgrammatiko
Copy link
Contributor

@akfaisel I thought that converting this script to Jquery would be a good thing. Yesterday. Today I realized that for some events people might be using mootools to program them, so unfortunately this cannot be done keeping backwards compatibility. Best way out of this is by using Bootstrap modal code...

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

5 participants