Moodle
  1. Moodle
  2. MDL-31740

Store form change state in a different js variable

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Duplicate
    • Affects Version/s: 2.3
    • Fix Version/s: DEV backlog
    • Component/s: Forms Library
    • Labels:
    • Affected Branches:
      MOODLE_23_STABLE

      Description

      As DanP pointed out in his comments on MDL-31655, M.cfg may not be the best place for storing the state information of form changes.
      I'm not aware of another suitably named variable in the JS namespace, so if we were to update the existing code from MDL-31315, MDL-31655, and MDL-31673 then we'd need a variable for this such as statedata:

      M.statedata = M.statedata || {};

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            Andrew Nicols added a comment -

            Sam,

            Dan thought it would be good to get your input on this as you reviewed the original commits.

            Show
            Andrew Nicols added a comment - Sam, Dan thought it would be good to get your input on this as you reviewed the original commits.
            Hide
            Sam Hemelryk added a comment -

            Hi Andrew,

            Typically with JS modules we create a namespace off of M that can be used for any methods and properties required by the module.
            In the case of lib/yui/formslib/formslib.js presently you are just using M.core.
            What I'd suggest is moving that code to a separate namespace (would be good to rename it at the same time) and end up with something like the following:

            M.core_formchangechecker = M.core_formchangechecker || {};
            M.core_formchangechecker.init = function(config) {}
            M.core_formchangechecker.stateinformation = [];
            

            Depending upon how far you wish to take it perhaps storing the FORMSLIB instances and having them store state would be the other way to do it.
            Then you could create a FORMSLIB.check_state method that you call for each instance.
            Of course then M.core_formchangechecker.stateinformation would become M.core_formchangechecker.instances.

            Anyway thats my thoughts on it.
            The big thing is I think we should create a namespace for all of this related code including the properties and use that.

            Cheers
            Sam

            Show
            Sam Hemelryk added a comment - Hi Andrew, Typically with JS modules we create a namespace off of M that can be used for any methods and properties required by the module. In the case of lib/yui/formslib/formslib.js presently you are just using M.core. What I'd suggest is moving that code to a separate namespace (would be good to rename it at the same time) and end up with something like the following: M.core_formchangechecker = M.core_formchangechecker || {}; M.core_formchangechecker.init = function(config) {} M.core_formchangechecker.stateinformation = []; Depending upon how far you wish to take it perhaps storing the FORMSLIB instances and having them store state would be the other way to do it. Then you could create a FORMSLIB.check_state method that you call for each instance. Of course then M.core_formchangechecker.stateinformation would become M.core_formchangechecker.instances. Anyway thats my thoughts on it. The big thing is I think we should create a namespace for all of this related code including the properties and use that. Cheers Sam
            Hide
            Michael de Raadt added a comment -

            Thanks for working with us on this, Andrew.

            Show
            Michael de Raadt added a comment - Thanks for working with us on this, Andrew.
            Hide
            Andrew Nicols added a comment -

            Closing this as a duplicate of MDL-31660 as I've merged the suggested changes in to that fix

            Show
            Andrew Nicols added a comment - Closing this as a duplicate of MDL-31660 as I've merged the suggested changes in to that fix

              People

              • Votes:
                0 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: