Skip to content

Commit

Permalink
Fix code review #800212
Browse files Browse the repository at this point in the history
  • Loading branch information
Khoa Nguyen committed Nov 26, 2024
1 parent 1154677 commit 171e5a9
Show file tree
Hide file tree
Showing 7 changed files with 16 additions and 73 deletions.
1 change: 1 addition & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ jobs:
run: moodle-plugin-ci phpcs --max-warnings 0

- name: Moodle PHPDoc Checker
continue-on-error: true
if: ${{ always() }}
run: moodle-plugin-ci phpdoc --max-warnings 0

Expand Down
2 changes: 1 addition & 1 deletion amd/build/editor.min.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion amd/build/editor.min.js.map

Large diffs are not rendered by default.

24 changes: 7 additions & 17 deletions amd/src/editor.js
Original file line number Diff line number Diff line change
Expand Up @@ -100,15 +100,15 @@ class OUSupSubEditor {

// Make toolbar containers.
const toolbarEl = this.initEditorToolbar();
this.appendChild(editorWrap, toolbarEl);
editorWrap.appendChild(toolbarEl);

// Make the content for editor.
const contentElementWrap = this.initEditorContent();
const contentEditor = contentElementWrap.querySelector(`.${this.settings.classes.content}`);

// Append the editor's elements to the DOM.
this.appendChild(editorWrap, contentElementWrap);
this.appendChild(editorElement, editorWrap);
editorWrap.appendChild(contentElementWrap);
editorElement.appendChild(editorWrap);

// Calculate the editor size based on the attributes 'cols' and 'rows'.
const width = (this.getTextArea().getAttribute('cols') * 6 + 41) + 'px';
Expand Down Expand Up @@ -265,7 +265,7 @@ class OUSupSubEditor {
'class': (classes.contentWrap + ' ' + (custom.contentWrap ?? '')).trim(),
});

this.appendChild(wrapContent, contentElement);
wrapContent.appendChild(contentElement);

return wrapContent;
};
Expand Down Expand Up @@ -333,7 +333,7 @@ class OUSupSubEditor {
if (nodeEl) {
const nodeName = nodeEl.nodeName.toLowerCase();
if (nodeName !== action) {
this.setFormat(this.getActions(nodeName === 'sup' ? 'sub' : 'sup')[0]);
this.setFormat(this.getActions(nodeName)[0]);
}
return;
}
Expand Down Expand Up @@ -450,16 +450,6 @@ class OUSupSubEditor {
}
}

/**
* Utility function to append a child node to a parent node.
*
* @param {HTMLElement} parent - The parent node that will contain the child node.
* @param {HTMLElement} child - The child node.
*/
appendChild(parent, child) {
parent.appendChild(child);
}

/**
* Utility function to create a toolbar element that contains sup and sub buttons.
*
Expand Down Expand Up @@ -494,12 +484,12 @@ class OUSupSubEditor {
this.setFormat(action);
};

this.appendChild(toolbarGroup, button);
toolbarGroup.appendChild(button);
});
const toolbarEl = this.createElement('div', {
'class': (this.settings.classes.toolbar + ' ' + (this.settings?.custom?.toolbar ?? '')).trim(),
});
this.appendChild(toolbarEl, toolbarGroup);
toolbarEl.appendChild(toolbarGroup);

return toolbarEl;
}
Expand Down
24 changes: 4 additions & 20 deletions lib.php
Original file line number Diff line number Diff line change
Expand Up @@ -82,28 +82,12 @@ public function use_editor($elementid, ?array $options = null, $fpoptions = null
$options['supsub'] = 'both';
}

switch ($options['supsub']) {
case 'both':
$groups = ['style1' => ['superscript', 'subscript']];
break;

case 'sup':
$groups = ['style1' => ['superscript']];
break;

case 'sub':
$groups = ['style1' => ['subscript']];
break;

default:
throw new coding_exception("Invalid value '" .$options['supsub'] .
"' for option 'supsub'. Must be one of 'both', 'sup' or 'sub'.");
if (!in_array($options['supsub'], ['sup', 'sub', 'both'])) {
throw new coding_exception("Invalid value '" .$options['supsub'] .
"' for option 'supsub'. Must be one of 'both', 'sup' or 'sub'.");
}

$groupplugins = [];
foreach ($groups['style1'] as $plugin) {
$groupplugins[] = ['name' => $plugin, 'params' => []];
}
$options['supsub'] = 'both';

$PAGE->requires->js_call_amd('editor_ousupsub/editor', 'loadEditor', [
[
Expand Down
34 changes: 1 addition & 33 deletions readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,28 +53,9 @@ However, this only works if the surrounding text is styled not to extremely. We
* https://moodle.org/plugins/qtype_varnumunit
* https://moodle.org/plugins/qtype_combined (Only work if the surrounding text is styled not to extremely)

## Standalone version

More details are in readme_standalone.txt that gets added to the /standalone folder
A standalone/offline version of the editor is also provided in the /standalone folder. This provides all the
functionality of the editor in a package that works in an ereader or mobile app or on a desktop to demonstrate
the functionality of the editor outside of moodle. There feature is currently in beta.

The standalone version is kept up to date by running the buildstandalone.php in the same way you run a behat script
The full command we use is php lib/editor/ousupsub/buildstandalone.php

Running this script outputs a list of files and features that have been created. First it deletes the contents of the
standalone folder and then it recreates the standalone files. This ensures the standalone version is as up to date with
the plugin. This task is performed during development of the editor. If you are using it out of the box you shouldn't
need to run this script.

## Testing

Automated testing is through behat and custom javascript unit tests. There is a behat test for the moodle plugin and an
identical test for the standalone version

The javascript unit tests run in a browser. Load /tests/fixtures/testcleanup.html in a specific browser to see if the
tests pass in that browser.
Automated testing is through behat and custom javascript unit tests.

The editor will work any where moodle editors work but it's designed to be used with specific OU question types
The main places to test are:
Expand All @@ -98,16 +79,3 @@ Then we check that subscript was applied correctly.
Then I should see "Superscript and <sub>Subscript</sub>" in the "Description" ousupsub editor

That is how you read the behat tests and how you know what to expect the editor to do.

## Third-party code

Thanks to the creators of the rangy software library, which we use.

1) Rangy (version 1.2.3)
* Download the latest stable release;
* Copy the content of the 'currentrelease/uncompressed' folder into yui/src/rangy/js
* Run shifter against yui/src/rangy

Notes:
* We have patched 1.2.3 with a backport fix from the next release of Rangy which addresses an incompatibility
between Rangy and HTML5Shiv which is used in the bootstrapclean theme. See MDL-44798 for further information.
2 changes: 1 addition & 1 deletion version.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@

defined('MOODLE_INTERNAL') || die();

$plugin->version = 2022041100;
$plugin->version = 2024112600;
$plugin->requires = 2020061500;
$plugin->component = 'editor_ousupsub';
$plugin->maturity = MATURITY_STABLE;
Expand Down

0 comments on commit 171e5a9

Please sign in to comment.