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
    • Rank:
      38336

      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 || {};
      

        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: