Moodle

Clean up grader report JavaScript

Details

  • Type: Sub-task Sub-task
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Fixed
  • Affects Version/s: 2.0
  • Fix Version/s: 2.0
  • Component/s: Gradebook
  • Labels:
    None
  • Affected Branches:
    MOODLE_20_STABLE
  • Fixed Branches:
    MOODLE_20_STABLE

Description

I know the gradebook was probably written first, but the JavaScript really needs to be updated to match http://docs.moodle.org/en/Development:JavaScript_guidelines. In particular

1. Putting Moodle code in the YAHOO namespace seems evil to me.
2. Almost all the code should be in external JS files not in the HTML, there seems to be a lot of JS in grade/report/grader/index.php
3. Related to that, the JS should be includable from the page footer, but becuase of the inline script, it isn't. See the todo on lnie 119 of grade/report/grader/index.php.

I was hoping you could fix these problems more easily than me, so assigning to you Nico, I hope that is OK.

Issue Links

This issue will be resolved by:
MDL-21058 Should moodle be adding JS to Yahoo's namespace? Minor Open

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Matt Gibson added a comment -

Tim, re: 1...

Is this (or will it soon be) an overall coding guideline type of thing? I've been adding more YAHOO namespaced stuff and also was about to start for my own projects. It seemed to be a good idea as the YAHOO.namespace() function checks for collisions and acts as a nice substitute for using globals.

Should I avoid this?

Show
Matt Gibson added a comment - Tim, re: 1... Is this (or will it soon be) an overall coding guideline type of thing? I've been adding more YAHOO namespaced stuff and also was about to start for my own projects. It seemed to be a good idea as the YAHOO.namespace() function checks for collisions and acts as a nice substitute for using globals. Should I avoid this?
Hide
Permalink
Matt Gibson added a comment -

Tim, re: 1...

Is this (or will it soon be) an overall coding guideline type of thing? I've been adding more YAHOO namespaced stuff and also was about to start for my own projects. It seemed to be a good idea as the YAHOO.namespace() function checks for collisions and acts as a nice substitute for using globals.

Should I avoid this?

Show
Matt Gibson added a comment - Tim, re: 1... Is this (or will it soon be) an overall coding guideline type of thing? I've been adding more YAHOO namespaced stuff and also was about to start for my own projects. It seemed to be a good idea as the YAHOO.namespace() function checks for collisions and acts as a nice substitute for using globals. Should I avoid this?
Hide
Permalink
Tim Hunt added a comment -

The whole point of a name space is for different libraries to keep everything in their own private area, so names do not collide.

Therefore, it is definitely wrong for us to shove stuff into the YAHOO namespace.

The docs page I linked to above is now part of the coding guidelines.

Show
Tim Hunt added a comment - The whole point of a name space is for different libraries to keep everything in their own private area, so names do not collide. Therefore, it is definitely wrong for us to shove stuff into the YAHOO namespace. The docs page I linked to above is now part of the coding guidelines.
Hide
Permalink
Matt Gibson added a comment -

Cool.

Thanks.

Show
Matt Gibson added a comment - Cool. Thanks.
Hide
Permalink
Andrew Davis added a comment -

The javascript has already been shifted into a .js file which is now appearing at the bottom of the page.

That only leaves the namespacing issue. I'll raise this at the developer meeting tuesday. If we are to move stuff out of the Yahoo namespace we need to decide on a way to do that throughout all of Moodle's JS.

Show
Andrew Davis added a comment - The javascript has already been shifted into a .js file which is now appearing at the bottom of the page. That only leaves the namespacing issue. I'll raise this at the developer meeting tuesday. If we are to move stuff out of the Yahoo namespace we need to decide on a way to do that throughout all of Moodle's JS.
Hide
Permalink
Tim Hunt added a comment -

Actually, this was the only bit of Moodle JS code that was polluting this YAHOO namespace (I think).

I think i wrote a bit about namespacing when I wrote http://docs.moodle.org/en/Development:JavaScript_guidelines - but that was about a year ago. There was a thread, I think in the General Developer Forum at about the same time, and there was some discussion of name-spacing then, if I recall.

Anyway, at the moment we are not really worrying about hiding Moodle JS within one namespace, we are putting it in the global namespace. That is not such a bad idea, since Moodle is the whole application. If someone is looking at a Moodle page then they are looking at a Moodle page, and we control all the JS there. This is the opposite to the YUI situation. That can be embedded anywhere, and so it protects YUI, and avoids clashing with other code for the whole of YUI to live in the YAHOO namespace.

My personal view is that requiring all Moodle JS to live inside Moodle. (or something) would just add noise, and make our code harder to read, for very little gain. However, I am prepared to be proved wrong about that.

A more interesting case is two third party plugins that try to define the same JS function. Namespacing by Moodle plugin might be worth considering. Anyway, please do raise this again, particularly if you have a good, practical suggestion.

Show
Tim Hunt added a comment - Actually, this was the only bit of Moodle JS code that was polluting this YAHOO namespace (I think). I think i wrote a bit about namespacing when I wrote http://docs.moodle.org/en/Development:JavaScript_guidelines - but that was about a year ago. There was a thread, I think in the General Developer Forum at about the same time, and there was some discussion of name-spacing then, if I recall. Anyway, at the moment we are not really worrying about hiding Moodle JS within one namespace, we are putting it in the global namespace. That is not such a bad idea, since Moodle is the whole application. If someone is looking at a Moodle page then they are looking at a Moodle page, and we control all the JS there. This is the opposite to the YUI situation. That can be embedded anywhere, and so it protects YUI, and avoids clashing with other code for the whole of YUI to live in the YAHOO namespace. My personal view is that requiring all Moodle JS to live inside Moodle. (or something) would just add noise, and make our code harder to read, for very little gain. However, I am prepared to be proved wrong about that. A more interesting case is two third party plugins that try to define the same JS function. Namespacing by Moodle plugin might be worth considering. Anyway, please do raise this again, particularly if you have a good, practical suggestion.
Hide
Permalink
Matt Gibson added a comment -

As I remember, using the YAHOO namespace involves calling a YUI function call that automatically checks for collisions and flags them up with an error. This was the only reason I thought that using it was a better idea than the global namespace. Otherwise, all the points raised above seem very sensible.

Show
Matt Gibson added a comment - As I remember, using the YAHOO namespace involves calling a YUI function call that automatically checks for collisions and flags them up with an error. This was the only reason I thought that using it was a better idea than the global namespace. Otherwise, all the points raised above seem very sensible.
Hide
Permalink
Andrew Davis added a comment -

I'm marking this issue resolved as 2 of the 3 issues raised have been dealt with. I'll open a new issue and link it to this one for the yahoo namespacing issue.

Show
Andrew Davis added a comment - I'm marking this issue resolved as 2 of the 3 issues raised have been dealt with. I'll open a new issue and link it to this one for the yahoo namespacing issue.

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved:
Atlassian JIRA (v4.4.3#663-r165197) | Report a problem
Powered by a free Atlassian JIRA open source license for Moodle. Try JIRA - bug tracking software for your team.