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

Create JHtml::_('behavior.core') to decouble core.js from mootools framework #2696

Closed
wants to merge 3 commits into from

Conversation

Bakual
Copy link
Contributor

@Bakual Bakual commented Dec 18, 2013

Issue

Currently, when we want to load core.js (kind of our own Joomla JavaScript "library") we use the same function as when we want to load the MooTools JavaScript library. That is JHtml::_('behavior.framework').
So far this wasn't a problem since core.js needed MooTools anyway.
Since we want to remove MooTools and there are quite a few PRs around which deals with that and also rewrites core.js to jQuery, we may end up loading MooTools only to get core.js. This is of course not good.

Proposed Solution

This PR will introduce a new function JHtml::_('behavior.core') which is supposed to only load core.js.
Currently, this would still load MooTools since core.js depends on it. It's implemented as a simple proxy to behavior.framework for now.
As soon as core.js is rewritten to jQuery, the function can be changed to load jquery.framework and core.js instead.
I already wrote the code for that with comments what needs to be done.

Goal

Introducing this new class already "ahead of time" would allow us to start rewriting our extensions to use this new class, so when core.js is changed, we don't need to change anything anymore.
After all core javascript functions are rewritten to jQuery, we can then deprecate behavior.framework and remove it with J4.0.
This also gives 3rd party developers a bit more time to do the transition.

Tracker

http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemEdit&tracker_item_id=32994

Bakual referenced this pull request Dec 19, 2013
Second parameter in [script] function should be now [false] for [tabstate] function. This is because this tabs-state.js requires JQuery not full Mootools framework. Setting this to [true] not only load Mootools when it is not required but also cause some conflicts if websites that depends on JQuery without noConflict.
@beat
Copy link
Contributor

beat commented Dec 19, 2013

Nice and needed decoupling.

However, if JHtml::_('behavior.core') is not loaded systematically in Joomla's admin area, or at least in the tabs of admin area in next 3.2.x, it will not fix the side-effect-incompatibility introduced by 0247d9f in 3.2.1.

@Bakual
Copy link
Contributor Author

Bakual commented Dec 19, 2013

I don't think we need to load this on each page in backend. It's the same as for jQuery, load it if needed. Don't load it if you don't need it.
We may of course end up needing it everywhere. I don't know yet the use of each function in it. Most seem to be related to lists and forms. Plus some function which deal with text and messages.

@Bakual
Copy link
Contributor Author

Bakual commented Dec 19, 2013

List of all functions currently in core.js for reference

  • Joomla "namespace" object
  • Joomla.editors -> An object to hold each editor instance on page
  • Joomla.submitform -> Generic submit form
  • Joomla.submitbutton -> Default function. Usually would be overriden by the component
  • Joomla.JText -> Allows you to call Joomla.JText._() to get a translated JavaScript string pushed in with JText::script() in Joomla.
  • Joomla.replaceTokens -> Method to replace all request tokens on the page with a new one.
  • Joomla.isEmail -> Verifies if the string is in a valid email format
  • Joomla.checkAll -> Toggles the check state of a group of boxes
  • Joomla.renderMessages -> Render messages send via JSON
  • Joomla.removeMessages -> Remove messages
  • Joomla.isChecked -> grid stuff
  • Joomla.popupWindow -> Pops up a new window in the middle of the screen (for help button)
  • Joomla.tableOrdering -> grid stuff
  • writeDynaList -> Writes a dynamically generated list
  • changeDynaList -> Changes a dynamically generated list
  • radioGetCheckedValue -> return the value of the radio button that is checked
  • getSelectedValue -> ?
  • listItemTask -> ?
  • submitbutton -> deprecated for Joomla.submitbutton
  • submitform -> deprecated for Joomla.submitform
  • saveorder -> needed for Table Column ordering
  • checkAll_button -> ?

@Fedik
Copy link
Member

Fedik commented Dec 19, 2013

I would be happy keep core.js without any library dependency, and load it over all scripts, when addScript or addScriptDeclaration called ... and then this will perfectly fit in #1260 , and avoid stupid thing with the language, like we have now

if I right remember there only ...Messages methods has such dependency...
also some methods (like a grid stuff) can be removed from core.js and moved in the own file.... but it just a thoughts

@Bakual
Copy link
Contributor Author

Bakual commented Dec 19, 2013

I wouldn't load it always. Especially in frontend most of the time it is not needed at all.

@betweenbrain
Copy link
Contributor

I wouldn't load it always. Especially in frontend most of the time it is not needed at all.

Yes, please don't.

@Fedik
Copy link
Member

Fedik commented Dec 20, 2013

I did not say "always",
I just say, only when any script used...

@Bakual
Copy link
Contributor Author

Bakual commented Dec 20, 2013

I did not say "always",
I just say, only when any script used...

Yes, but the result will be that it's loaded always as most sites have at least one script loaded on a page.
But why should we load it together with any scripts? It's not needed at all in most cases. Especially in frontend.

@Fedik
Copy link
Member

Fedik commented Dec 20, 2013

ideas come from #1260 and #1319
this allow to make sure that Joomla object with the core methods always available,
this allow to avoid duplication like we have with "script language", where Joomla! render the additional code because no one know whether there core.js is loaded or not,
for #1319 I wanted add .extend() method but I cannot do that because I cannot be sure that core.js loaded ...
of course I can call behaviour.framework but this will load additional library that not always need...

but main idea of my first comment here was: would be good to have the core.js without library dependency

@Bakual
Copy link
Contributor Author

Bakual commented Dec 20, 2013

of course I can call behaviour.framework but this will load additional library that not always need...

Currently, core.js needs MooTools for its functions. That's why the additional library is loaded. It's always needed when you load core.js.

It's exactly the purpose of this PR to decouple the loading from core.js from MooTools.
If core.js is rewritten to jQuery, we will need the function proposed here to load core.js without MooTools.
However it will then of course load jQuery as it will need that library instead.

@Fedik
Copy link
Member

Fedik commented Dec 20, 2013

I can try change the script in core.js for remove library dependency, if we will agree on that point :)

@Bakual
Copy link
Contributor Author

Bakual commented Dec 20, 2013

@Fedik There is already a PR to change it to jQuery (linked above). I'd suggest contacting Ashan Fernando if it can be improved.
However I would still not load core.js as soon as another script is loaded. 😃

@Bakual
Copy link
Contributor Author

Bakual commented Feb 4, 2014

New one against staging: #3047

@Bakual Bakual deleted the CreateBehaviorCore branch May 12, 2014 07:43
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

4 participants