Review - Rubric content type

Description

Please review the new content type created by pjotr.savitski.

Posted in the forums: https://h5p.org/node/506057/

Acceptance Criteria

None

Activity

Show:
Thomas Marstrander
August 1, 2019, 11:05 AM

Rubric review:

In general looks very good functionally, could use a fresher design and responsiveness. I’ll list the functional things I’ve noted down, which you might want to have a look at.

Overview:

  • External JS libraries loaded from CDN should be downloaded and included directly within the used content type

    • This is both more stable (prevents the file from being updated with breaking changes), and more safe and compatible with self-hosted servers (prevents xss)

  • The content type should use the embed type iFrame, instead of 'div' (unless there is a very specific reason for using div), so it prevents the content type from leaking styles, overwriting global libraries, having direct access to the parent site, etc

  • Special characters are not rendered as html, they are not decoded.

  • It is a quite hard to understand what questions this content type can be used for and how it can be used. You could add some more descriptions and placeholders to help the author get started.

Code review:
h5p-editor-rubric:

  • please add LICENSE to .h5pignore, so that it won't be stopped by the extensions whitelist of h5p-core

h5p-rubric:

  • please add LICENSE to .h5pignore, so that it won't be stopped by the extensions whitelist of h5p-core

  • All codelines start at 1 space indentation, please refer to our coding style guide for proper indentations: https://h5p.org/code-style and run our eslint config for your projects found at https://github.com/h5p/h5p-dev-tools.

Accessibility:

  • Rubrics can not be selected by keyboard

  • Some WAI-ARIA meta information for the table would be usefull for screen readers

 

Assignee

Thomas Marstrander

Reporter

Bienvenido Villadelrey

Labels

None

Funding

None

Code reviewer

None

Released

None

Time tracking

5h

Fix versions

Priority

Medium
Configure