Uploaded image for project: 'Plugins'
  1. Plugins
  2. CONTRIB-1778

Code review comments


    • Icon: Bug Bug
    • Resolution: Fixed
    • Icon: Minor Minor
    • None
    • 1.9.7
    • None

      Looks like really good work.

      Some minor comments from looking at the code:

      • There are much better ways to get data from PHP to JS than the one you use in question.html. Try

        $flashvars = array(
            'direction' => $question->options->direction,
            'quality' => $question->options->quality,
            // etc.
            'responseFunc' => 'setResponse' . $nameprefix,
        <script type="text/javascript">
            var flashvars = <?php echo json_encode($flashvars); ?>

      • Why does inMoodle need to be a function, not a flashvar? and is getState actually used?
      • Would be nice to spell out the interval types Major, Minor, etc. with lang strings on the settings form.
      • Did you try using the right unicode character for flat, etc. in that settings dropdown?
      • Should you have a readme file in the swfobject folder to acknowledge that it is third-party code?
      • question/type/interval/lang/en_utf8/help/interval/interval.html should use <h1> for the title.
      • Moodle coding guideline is 4 spaces for indent, not tabs.
      • I normally double-indent the following lines when wrapping a line of code onto multiple lines.

      As a cool idea for the future. It would be nice for the teacher if you could use the same flash applet on the edit form, for the teacher to enter the starting note. You probably already thought of that.

      Hope that is helpful.

      Tim. Quiz module / Question bank maintainer.

            brissone Eric Brisson
            timhunt Tim Hunt
            0 Vote for this issue
            0 Start watching this issue


                Error rendering 'clockify-timesheets-time-tracking-reports:timer-sidebar'. Please contact your Jira administrators.