From 0a23dc043d0ab3195a5db24470235d851e322600 Mon Sep 17 00:00:00 2001 From: Paal Joergensen Date: Tue, 13 Dec 2016 10:29:33 +0100 Subject: [PATCH 1/9] Download and embed granularity (HFP-277) --- h5p.classes.php | 142 +++++++++++++++++++++++++++++--------- js/disable.js | 19 ----- js/h5p-action-bar.js | 77 +++++++++++++++++++++ js/h5p-display-options.js | 23 ++++++ js/h5p.js | 103 +++++++-------------------- 5 files changed, 236 insertions(+), 128 deletions(-) delete mode 100644 js/disable.js create mode 100644 js/h5p-action-bar.js create mode 100644 js/h5p-display-options.js diff --git a/h5p.classes.php b/h5p.classes.php index dabca7d..5f8a10a 100644 --- a/h5p.classes.php +++ b/h5p.classes.php @@ -558,6 +558,8 @@ interface H5PFrameworkInterface { * Will trigger after the export file is created. */ public function afterExportCreated(); + + public function hasPermission($permission, $content_id = NULL); } /** @@ -1669,6 +1671,20 @@ Class H5PExport { } } +abstract class H5PPermission { + const DOWNLOAD_H5P = 0; + const EMBED_H5P = 1; +} + +abstract class H5PDisplayOptionBehaviour { + const NEVER_SHOW = 0; + const CONTROLLED_BY_AUTHOR_DEFAULT_ON = 1; + const CONTROLLED_BY_AUTHOR_DEFAULT_OFF = 2; + const ALWAYS_SHOW = 3; + const CONTROLLED_BY_PERMISSIONS = 4; +} + + /** * Functions and storage shared by the other H5P classes */ @@ -1690,7 +1706,8 @@ class H5PCore { 'js/h5p-x-api-event.js', 'js/h5p-x-api.js', 'js/h5p-content-type.js', - 'js/h5p-confirmation-dialog.js' + 'js/h5p-confirmation-dialog.js', + 'js/h5p-action-bar.js' ); public static $adminScripts = array( 'js/jquery.js', @@ -2439,34 +2456,6 @@ class H5PCore { } } - /** - * - */ - public function getGlobalDisable() { - $disable = self::DISABLE_NONE; - - // Allow global settings to override and disable options - if (!$this->h5pF->getOption('frame', TRUE)) { - $disable |= self::DISABLE_FRAME; - } - else { - if (!$this->h5pF->getOption('export', TRUE)) { - $disable |= self::DISABLE_DOWNLOAD; - } - if (!$this->h5pF->getOption('embed', TRUE)) { - $disable |= self::DISABLE_EMBED; - } - if (!$this->h5pF->getOption('copyright', TRUE)) { - $disable |= self::DISABLE_COPYRIGHT; - } - if (!$this->h5pF->getOption('icon', TRUE)) { - $disable |= self::DISABLE_ABOUT; - } - } - - return $disable; - } - /** * Determine disable state from sources. * @@ -2474,7 +2463,7 @@ class H5PCore { * @param int $current * @return int */ - public function getDisable(&$sources, $current) { + public function getDisplayOptionsAsByte(&$sources, $current) { foreach (H5PCore::$disable as $bit => $option) { if ($this->h5pF->getOption(($bit & H5PCore::DISABLE_DOWNLOAD ? 'export' : $option), TRUE)) { if (!isset($sources[$option]) || !$sources[$option]) { @@ -2488,6 +2477,98 @@ class H5PCore { return $current; } + /** + * Determine display options visibility and value on edit + * + * @method getDisplayOptionsForEdit + * @param [int] $disable + * @return [Array] + */ + public function getDisplayOptionsForEdit($disable = NULL) { + $display_options = []; + + $current_display_options = $disable === NULL ? [] : $this->getDisplayOptionsAsArray($disable); + + if ($this->h5pF->getOption('frame', TRUE)) { + $display_options['frame'] = isset($current_display_options['showFrame']) ? $current_display_options['showFrame'] : TRUE; + + $export = $this->h5pF->getOption('export', H5PDisplayOptionBehaviour::ALWAYS_SHOW); + if ($export == H5PDisplayOptionBehaviour::CONTROLLED_BY_AUTHOR_DEFAULT_ON || $export == H5PDisplayOptionBehaviour::CONTROLLED_BY_AUTHOR_DEFAULT_OFF) { + $display_options['download'] = isset($current_display_options['showDownload']) ? $current_display_options['showDownload'] : ($export == H5PDisplayOptionBehaviour::CONTROLLED_BY_AUTHOR_DEFAULT_ON); + } + $embed = $this->h5pF->getOption('embed', H5PDisplayOptionBehaviour::ALWAYS_SHOW); + if ($embed == H5PDisplayOptionBehaviour::CONTROLLED_BY_AUTHOR_DEFAULT_ON || $embed == H5PDisplayOptionBehaviour::CONTROLLED_BY_AUTHOR_DEFAULT_OFF) { + $display_options['embed'] = isset($current_display_options['showEmbed']) ? $current_display_options['showEmbed'] : ($embed == H5PDisplayOptionBehaviour::CONTROLLED_BY_AUTHOR_DEFAULT_ON); + } + if ($this->h5pF->getOption('copyright', TRUE)) { + $display_options['copyright'] = isset($current_display_options['showCopyright']) ? $current_display_options['showCopyright'] : TRUE; + } + } + + return $display_options; + } + + /** + * Determine display option visibility when viewing H5P + * + * @method getDisplayOptionsForView + * @param [int] $display_options + * @param [int] $id Might be content id or user id. + * Depends on what the platform needs to be able to determine permissions. + * @return [Array] + */ + public function getDisplayOptionsForView($disable, $id) { + $display_options = $this->getDisplayOptionsAsArray($disable); + + if ($this->h5pF->getOption('frame', TRUE) == FALSE) { + $display_options['showFrame'] = false; + } + else { + $export = $this->h5pF->getOption('export', H5PDisplayOptionBehaviour::ALWAYS_SHOW); + if ($export == H5PDisplayOptionBehaviour::NEVER_SHOW) { + // If never show globally, force hide + $display_options['showDownload'] = false; + } + elseif ($export == H5PDisplayOptionBehaviour::ALWAYS_SHOW || ($export == H5PDisplayOptionBehaviour::CONTROLLED_BY_PERMISSIONS && $this->h5pF->hasPermission(H5PPermission::DOWNLOAD_H5P, $id))) { + // If always show or permissions say so, force show + $display_options['showDownload'] = true; + } + + $embed = $this->h5pF->getOption('embed', H5PDisplayOptionBehaviour::ALWAYS_SHOW); + if ($embed == H5PDisplayOptionBehaviour::NEVER_SHOW) { + // If never show globally, force hide + $display_options['showEmbed'] = false; + } + elseif ($embed == H5PDisplayOptionBehaviour::ALWAYS_SHOW || ($embed == H5PDisplayOptionBehaviour::CONTROLLED_BY_PERMISSIONS && $this->h5pF->hasPermission(H5PPermission::EMBED_H5P, $id))) { + // If always show or permissions say so, force show + $display_options['showEmbed'] = true; + } + + if ($this->h5pF->getOption('copyright', TRUE) == FALSE) { + $display_options['showCopyright'] = false; + } + } + + return $display_options; + } + + /** + * Convert display options as single byte to array + * + * @method getDisplayOptionsAsArray + * @param [int] $disable + * @return [Array] + */ + private function getDisplayOptionsAsArray($disable) { + return array( + 'showFrame' => !($disable & H5PCore::DISABLE_FRAME), + 'showDownload' => !($disable & H5PCore::DISABLE_DOWNLOAD), + 'showEmbed' => !($disable & H5PCore::DISABLE_EMBED), + 'showCopyright' => !($disable & H5PCore::DISABLE_COPYRIGHT), + 'showAbout' => $this->h5pF->getOption('icon', TRUE), + ); + } + /** * Small helper for getting the library's ID. * @@ -3147,7 +3228,6 @@ class H5PContentValidator { break; } } - // Using a library in content that is not present at all in semantics if ($message === NULL) { $message = $this->h5pF->t('The H5P library %library used in the content is not valid', array( diff --git a/js/disable.js b/js/disable.js deleted file mode 100644 index 0bfbb07..0000000 --- a/js/disable.js +++ /dev/null @@ -1,19 +0,0 @@ -(function ($) { - $(document).ready(function () { - var $inputs = $('.h5p-action-bar-settings input'); - var $frame = $inputs.filter('input[name="frame"], input[name="h5p_frame"]'); - var $others = $inputs.filter(':not(input[name="frame"], input[name="h5p_frame"])'); - - var toggle = function () { - if ($frame.is(':checked')) { - $others.attr('disabled', false); - } - else { - $others.attr('disabled', true); - } - }; - - $frame.change(toggle); - toggle(); - }); -})(H5P.jQuery); diff --git a/js/h5p-action-bar.js b/js/h5p-action-bar.js new file mode 100644 index 0000000..a80ef60 --- /dev/null +++ b/js/h5p-action-bar.js @@ -0,0 +1,77 @@ +H5P.ActionBar = (function ($, EventDispatcher) { + "use strict"; + + function ActionBar(displayOptions) { + EventDispatcher.call(this); + + var self = this; + + var hasActions = false; + + // Create action bar + var $actions = H5P.jQuery(''); + + /** + * Helper for creating action bar buttons. + * + * @private + * @param {string} type + * @param {string} customClass Instead of type class + */ + var addActionButton = function (type, customClass) { + var handler = function () { + self.trigger(type); + }; + H5P.jQuery('
  • ', { + 'class': 'h5p-button h5p-' + (customClass ? customClass : type), + role: 'button', + tabindex: 0, + title: H5P.t(type + 'Description'), + html: H5P.t(type), + on: { + click: handler, + keypress: function (e) { + if (e.which === 32) { + handler(); + e.preventDefault(); // (since return false will block other inputs) + } + } + }, + appendTo: $actions + }); + + hasActions = true; + }; + + // Register action bar buttons + if (displayOptions.showDownload) { + // Add export button + addActionButton('download', 'export'); + } + if (displayOptions.showCopyrights) { + addActionButton('copyrights'); + } + if (displayOptions.showEmbed) { + addActionButton('embed'); + } + if (displayOptions.showAbout) { + // Add about H5P button icon + H5P.jQuery('
    • ').appendTo($actions); + hasActions = true; + } + + self.getDOMElement = function () { + return $actions; + }; + + self.hasActions = function () { + return hasActions; + } + }; + + ActionBar.prototype = Object.create(EventDispatcher.prototype); + ActionBar.prototype.constructor = ActionBar; + + return ActionBar; + +})(H5P.jQuery, H5P.EventDispatcher); diff --git a/js/h5p-display-options.js b/js/h5p-display-options.js new file mode 100644 index 0000000..9c8f664 --- /dev/null +++ b/js/h5p-display-options.js @@ -0,0 +1,23 @@ +/** + * Utility that makes it possible to hide fields when a checkbox is unchecked + */ +(function ($) { + function setupHiding () { + var $toggler = $(this); + + // Getting the field which should be hidden: + var $subject = $($toggler.data('h5p-visibility-subject-selector')); + + var toggle = function () { + $subject.toggle($toggler.is(':checked')); + }; + + $toggler.change(toggle); + toggle(); + } + + $(document).ready(function () { + // Get the checkboxes making other fields being hidden: + $('.h5p-visibility-toggler').each(setupHiding); + }); +})(H5P.jQuery); diff --git a/js/h5p.js b/js/h5p.js index 77f9f0e..5fe8c78 100644 --- a/js/h5p.js +++ b/js/h5p.js @@ -47,24 +47,6 @@ else if (document.documentElement.msRequestFullscreen) { H5P.fullScreenBrowserPrefix = 'ms'; } -/** @const {number} */ -H5P.DISABLE_NONE = 0; - -/** @const {number} */ -H5P.DISABLE_FRAME = 1; - -/** @const {number} */ -H5P.DISABLE_DOWNLOAD = 2; - -/** @const {number} */ -H5P.DISABLE_EMBED = 4; - -/** @const {number} */ -H5P.DISABLE_COPYRIGHT = 8; - -/** @const {number} */ -H5P.DISABLE_ABOUT = 16; - /** * Keep track of when the H5Ps where started. * @@ -147,76 +129,41 @@ H5P.init = function (target) { }); } - // Create action bar - var $actions = H5P.jQuery(''); - /** - * Helper for creating action bar buttons. - * - * @private - * @param {string} type - * @param {function} handler - * @param {string} customClass Instead of type class + * Create action bar */ - var addActionButton = function (type, handler, customClass) { - H5P.jQuery('
  • ', { - 'class': 'h5p-button h5p-' + (customClass ? customClass : type), - role: 'button', - tabindex: 0, - title: H5P.t(type + 'Description'), - html: H5P.t(type), - on: { - click: handler, - keypress: function (e) { - if (e.which === 32) { - handler(); - e.preventDefault(); // (since return false will block other inputs) - } - } - }, - appendTo: $actions - }); - }; - - // Register action bar buttons - if (!(contentData.disable & H5P.DISABLE_DOWNLOAD)) { - // Add export button - addActionButton('download', function () { - // Use button for download to avoid people linking directly to the .h5p - window.location.href = contentData.exportUrl; - }, 'export'); - } - if (!(contentData.disable & H5P.DISABLE_COPYRIGHT)) { - var copyright = H5P.getCopyrights(instance, library.params, contentId); - - if (copyright) { - // Add copyright dialog button - addActionButton('copyrights', function () { - // Open dialog with copyright information - var dialog = new H5P.Dialog('copyrights', H5P.t('copyrightInformation'), copyright, $container); - dialog.open(); - }); + var displayOptions = contentData.displayOptions; + if (displayOptions.showFrame) { + // Special handling of copyrights + if (displayOptions.showCopyrights) { + var copyrights = H5P.getCopyrights(instance, library.params, contentId); + if (!copyrights) { + displayOptions.showCopyrights = false; + } } - } - if (!(contentData.disable & H5P.DISABLE_EMBED)) { - // Add embed button - addActionButton('embed', function () { - // Open dialog with embed information + + // Create action bar + var actionBar = new H5P.ActionBar(displayOptions); + var $actions = actionBar.getDOMElement(); + + actionBar.on('download', function () { + window.location.href = contentData.exportUrl; + }); + actionBar.on('copyrights', function () { + var dialog = new H5P.Dialog('copyrights', H5P.t('copyrightInformation'), copyrights, $container); + dialog.open(); + }); + actionBar.on('embed', function () { H5P.openEmbedDialog($actions, contentData.embedCode, contentData.resizeCode, { width: $element.width(), height: $element.height() }); }); - } - if (!(contentData.disable & H5P.DISABLE_ABOUT)) { - // Add about H5P button icon - H5P.jQuery('
    • ').appendTo($actions); - } - - // Insert action bar if it has any content - if (!(contentData.disable & H5P.DISABLE_FRAME) && $actions.children().length) { $actions.insertAfter($container); + } + + if (displayOptions.showFrame && actionBar.hasActions()) { $element.addClass('h5p-frame'); } else { From e69a81f4039d5c84fd53821a36d82e347a5479d4 Mon Sep 17 00:00:00 2001 From: Paal Joergensen Date: Tue, 13 Dec 2016 10:33:59 +0100 Subject: [PATCH 2/9] Added description of new interface method [HFP-277] --- h5p.classes.php | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/h5p.classes.php b/h5p.classes.php index 5f8a10a..ddb48b6 100644 --- a/h5p.classes.php +++ b/h5p.classes.php @@ -559,7 +559,15 @@ interface H5PFrameworkInterface { */ public function afterExportCreated(); - public function hasPermission($permission, $content_id = NULL); + /** + * Check if user has permissions to an action + * + * @method hasPermission + * @param [H5PPermission] $permission Permission type, ref H5PPermission + * @param [int] $id Id need by platform to determine permission + * @return boolean + */ + public function hasPermission($permission, $id = NULL); } /** From cfd6cb1e639b7f83bbf8a2d5cf5805ebc57a4046 Mon Sep 17 00:00:00 2001 From: Paal Joergensen Date: Tue, 13 Dec 2016 15:57:05 +0100 Subject: [PATCH 3/9] Fixed download/embed button bug when permissions are used --- h5p.classes.php | 45 ++++++++++++++++++++++++++------------------- 1 file changed, 26 insertions(+), 19 deletions(-) diff --git a/h5p.classes.php b/h5p.classes.php index ddb48b6..d32ee12 100644 --- a/h5p.classes.php +++ b/h5p.classes.php @@ -2516,6 +2516,30 @@ class H5PCore { return $display_options; } + /** + * Helper function used to figure out embed & download behaviour + * + * @method setDisplayOptionOverrides + * @param string $option_name + * @param H5PPermission $permission + * @param int $id + * @param bool &$value + */ + private function setDisplayOptionOverrides($option_name, $permission, $id, &$value) { + $behaviour = $this->h5pF->getOption($option_name, H5PDisplayOptionBehaviour::ALWAYS_SHOW); + // If never show globally, force hide + if ($behaviour == H5PDisplayOptionBehaviour::NEVER_SHOW) { + $value = false; + } + elseif ($behaviour == H5PDisplayOptionBehaviour::ALWAYS_SHOW) { + // If always show or permissions say so, force show + $value = true; + } + elseif ($behaviour == H5PDisplayOptionBehaviour::CONTROLLED_BY_PERMISSIONS) { + $value = $this->h5pF->hasPermission($permission, $id); + } + } + /** * Determine display option visibility when viewing H5P * @@ -2532,25 +2556,8 @@ class H5PCore { $display_options['showFrame'] = false; } else { - $export = $this->h5pF->getOption('export', H5PDisplayOptionBehaviour::ALWAYS_SHOW); - if ($export == H5PDisplayOptionBehaviour::NEVER_SHOW) { - // If never show globally, force hide - $display_options['showDownload'] = false; - } - elseif ($export == H5PDisplayOptionBehaviour::ALWAYS_SHOW || ($export == H5PDisplayOptionBehaviour::CONTROLLED_BY_PERMISSIONS && $this->h5pF->hasPermission(H5PPermission::DOWNLOAD_H5P, $id))) { - // If always show or permissions say so, force show - $display_options['showDownload'] = true; - } - - $embed = $this->h5pF->getOption('embed', H5PDisplayOptionBehaviour::ALWAYS_SHOW); - if ($embed == H5PDisplayOptionBehaviour::NEVER_SHOW) { - // If never show globally, force hide - $display_options['showEmbed'] = false; - } - elseif ($embed == H5PDisplayOptionBehaviour::ALWAYS_SHOW || ($embed == H5PDisplayOptionBehaviour::CONTROLLED_BY_PERMISSIONS && $this->h5pF->hasPermission(H5PPermission::EMBED_H5P, $id))) { - // If always show or permissions say so, force show - $display_options['showEmbed'] = true; - } + $this->setDisplayOptionOverrides('export', H5PPermission::DOWNLOAD_H5P, $id, $display_options['showDownload']); + $this->setDisplayOptionOverrides('embed', H5PPermission::EMBED_H5P, $id, $display_options['showEmbed']); if ($this->h5pF->getOption('copyright', TRUE) == FALSE) { $display_options['showCopyright'] = false; From 526d7ddd7e211eaa425bca6cf7b1ef5b6e5dff0b Mon Sep 17 00:00:00 2001 From: Paal Joergensen Date: Fri, 16 Dec 2016 11:30:59 +0100 Subject: [PATCH 4/9] Made copyright button being shown [HFP-277] --- js/h5p-action-bar.js | 2 +- js/h5p.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/js/h5p-action-bar.js b/js/h5p-action-bar.js index a80ef60..8d30101 100644 --- a/js/h5p-action-bar.js +++ b/js/h5p-action-bar.js @@ -48,7 +48,7 @@ H5P.ActionBar = (function ($, EventDispatcher) { // Add export button addActionButton('download', 'export'); } - if (displayOptions.showCopyrights) { + if (displayOptions.showCopyright) { addActionButton('copyrights'); } if (displayOptions.showEmbed) { diff --git a/js/h5p.js b/js/h5p.js index 5fe8c78..4af85e9 100644 --- a/js/h5p.js +++ b/js/h5p.js @@ -135,10 +135,10 @@ H5P.init = function (target) { var displayOptions = contentData.displayOptions; if (displayOptions.showFrame) { // Special handling of copyrights - if (displayOptions.showCopyrights) { + if (displayOptions.showCopyright) { var copyrights = H5P.getCopyrights(instance, library.params, contentId); if (!copyrights) { - displayOptions.showCopyrights = false; + displayOptions.showCopyright = false; } } From a1e68d212b907c58aa5e300cafa3cadeb0e4d771 Mon Sep 17 00:00:00 2001 From: Paal Joergensen Date: Fri, 16 Dec 2016 11:38:47 +0100 Subject: [PATCH 5/9] Updated comments [HFP-277] --- h5p.classes.php | 28 ++++++++++++---------------- js/h5p-action-bar.js | 12 ++++++++++++ 2 files changed, 24 insertions(+), 16 deletions(-) diff --git a/h5p.classes.php b/h5p.classes.php index d32ee12..7a71041 100644 --- a/h5p.classes.php +++ b/h5p.classes.php @@ -2488,9 +2488,8 @@ class H5PCore { /** * Determine display options visibility and value on edit * - * @method getDisplayOptionsForEdit - * @param [int] $disable - * @return [Array] + * @param int $disable + * @return array */ public function getDisplayOptionsForEdit($disable = NULL) { $display_options = []; @@ -2519,11 +2518,10 @@ class H5PCore { /** * Helper function used to figure out embed & download behaviour * - * @method setDisplayOptionOverrides - * @param string $option_name - * @param H5PPermission $permission - * @param int $id - * @param bool &$value + * @param string $option_name + * @param H5PPermission $permission + * @param int $id + * @param bool &$value */ private function setDisplayOptionOverrides($option_name, $permission, $id, &$value) { $behaviour = $this->h5pF->getOption($option_name, H5PDisplayOptionBehaviour::ALWAYS_SHOW); @@ -2543,11 +2541,10 @@ class H5PCore { /** * Determine display option visibility when viewing H5P * - * @method getDisplayOptionsForView - * @param [int] $display_options - * @param [int] $id Might be content id or user id. - * Depends on what the platform needs to be able to determine permissions. - * @return [Array] + * @param int $display_options + * @param int $id Might be content id or user id. + * Depends on what the platform needs to be able to determine permissions. + * @return array */ public function getDisplayOptionsForView($disable, $id) { $display_options = $this->getDisplayOptionsAsArray($disable); @@ -2570,9 +2567,8 @@ class H5PCore { /** * Convert display options as single byte to array * - * @method getDisplayOptionsAsArray - * @param [int] $disable - * @return [Array] + * @param int $disable + * @return array */ private function getDisplayOptionsAsArray($disable) { return array( diff --git a/js/h5p-action-bar.js b/js/h5p-action-bar.js index 8d30101..aba17e3 100644 --- a/js/h5p-action-bar.js +++ b/js/h5p-action-bar.js @@ -60,10 +60,22 @@ H5P.ActionBar = (function ($, EventDispatcher) { hasActions = true; } + /** + * Returns a reference to the dom element + * + * @method getDOMElement + * @return {H5P.jQuery} + */ self.getDOMElement = function () { return $actions; }; + /** + * Does the actionbar contain actions? + * + * @method hasActions + * @return {Boolean} + */ self.hasActions = function () { return hasActions; } From ea0362c3da73e5a3d5aa3b731a22adbc7f4890b8 Mon Sep 17 00:00:00 2001 From: Paal Joergensen Date: Fri, 16 Dec 2016 13:27:40 +0100 Subject: [PATCH 6/9] Making lines shorter [HFP-277] --- h5p.classes.php | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/h5p.classes.php b/h5p.classes.php index 7a71041..302242e 100644 --- a/h5p.classes.php +++ b/h5p.classes.php @@ -2499,16 +2499,32 @@ class H5PCore { if ($this->h5pF->getOption('frame', TRUE)) { $display_options['frame'] = isset($current_display_options['showFrame']) ? $current_display_options['showFrame'] : TRUE; + // Download $export = $this->h5pF->getOption('export', H5PDisplayOptionBehaviour::ALWAYS_SHOW); - if ($export == H5PDisplayOptionBehaviour::CONTROLLED_BY_AUTHOR_DEFAULT_ON || $export == H5PDisplayOptionBehaviour::CONTROLLED_BY_AUTHOR_DEFAULT_OFF) { - $display_options['download'] = isset($current_display_options['showDownload']) ? $current_display_options['showDownload'] : ($export == H5PDisplayOptionBehaviour::CONTROLLED_BY_AUTHOR_DEFAULT_ON); + if ($export == H5PDisplayOptionBehaviour::CONTROLLED_BY_AUTHOR_DEFAULT_ON || + $export == H5PDisplayOptionBehaviour::CONTROLLED_BY_AUTHOR_DEFAULT_OFF) { + $display_options['download'] = + isset($current_display_options['showDownload']) ? + $current_display_options['showDownload'] : + ($export == H5PDisplayOptionBehaviour::CONTROLLED_BY_AUTHOR_DEFAULT_ON); } + + // Embed $embed = $this->h5pF->getOption('embed', H5PDisplayOptionBehaviour::ALWAYS_SHOW); - if ($embed == H5PDisplayOptionBehaviour::CONTROLLED_BY_AUTHOR_DEFAULT_ON || $embed == H5PDisplayOptionBehaviour::CONTROLLED_BY_AUTHOR_DEFAULT_OFF) { - $display_options['embed'] = isset($current_display_options['showEmbed']) ? $current_display_options['showEmbed'] : ($embed == H5PDisplayOptionBehaviour::CONTROLLED_BY_AUTHOR_DEFAULT_ON); + if ($embed == H5PDisplayOptionBehaviour::CONTROLLED_BY_AUTHOR_DEFAULT_ON || + $embed == H5PDisplayOptionBehaviour::CONTROLLED_BY_AUTHOR_DEFAULT_OFF) { + $display_options['embed'] = + isset($current_display_options['showEmbed']) ? + $current_display_options['showEmbed'] : + ($embed == H5PDisplayOptionBehaviour::CONTROLLED_BY_AUTHOR_DEFAULT_ON); } + + // Copyright if ($this->h5pF->getOption('copyright', TRUE)) { - $display_options['copyright'] = isset($current_display_options['showCopyright']) ? $current_display_options['showCopyright'] : TRUE; + $display_options['copyright'] = + isset($current_display_options['showCopyright']) ? + $current_display_options['showCopyright'] : + TRUE; } } From 7b39a6900c5ca330dabd14a123ee070b67c674d5 Mon Sep 17 00:00:00 2001 From: Paal Joergensen Date: Fri, 16 Dec 2016 13:29:18 +0100 Subject: [PATCH 7/9] Inheriting global disable setting for export & embed when saving [HFP-277] --- h5p.classes.php | 30 +++++++++++++++++++++--------- 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/h5p.classes.php b/h5p.classes.php index 302242e..08f2a8e 100644 --- a/h5p.classes.php +++ b/h5p.classes.php @@ -2465,21 +2465,33 @@ class H5PCore { } /** - * Determine disable state from sources. + * Create representation of display options as int * * @param array $sources * @param int $current * @return int */ - public function getDisplayOptionsAsByte(&$sources, $current) { + public function getStorableDisplayOptions(&$sources, $current) { + // Download - force setting it if always on or always off + $export = $this->h5pF->getOption('export', H5PDisplayOptionBehaviour::ALWAYS_SHOW); + if ($export == H5PDisplayOptionBehaviour::ALWAYS_SHOW || + $export == H5PDisplayOptionBehaviour::NEVER_SHOW) { + $sources['download'] = ($export == H5PDisplayOptionBehaviour::ALWAYS_SHOW); + } + + // Embed - force setting it if always on or always off + $embed = $this->h5pF->getOption('embed', H5PDisplayOptionBehaviour::ALWAYS_SHOW); + if ($embed == H5PDisplayOptionBehaviour::ALWAYS_SHOW || + $embed == H5PDisplayOptionBehaviour::NEVER_SHOW) { + $sources['embed'] = ($embed == H5PDisplayOptionBehaviour::ALWAYS_SHOW); + } + foreach (H5PCore::$disable as $bit => $option) { - if ($this->h5pF->getOption(($bit & H5PCore::DISABLE_DOWNLOAD ? 'export' : $option), TRUE)) { - if (!isset($sources[$option]) || !$sources[$option]) { - $current |= $bit; // Disable - } - else { - $current &= ~$bit; // Enable - } + if (!isset($sources[$option]) || !$sources[$option]) { + $current |= $bit; // Disable + } + else { + $current &= ~$bit; // Enable } } return $current; From 87ec43d687f21903b5332ee853ede13adcfed7e0 Mon Sep 17 00:00:00 2001 From: Paal Joergensen Date: Fri, 16 Dec 2016 14:22:03 +0100 Subject: [PATCH 8/9] Using constants for the different display options, and avoid special cases [HFP-277] --- h5p.classes.php | 79 ++++++++++++++++++++++++-------------------- js/h5p-action-bar.js | 8 ++--- js/h5p.js | 8 ++--- 3 files changed, 52 insertions(+), 43 deletions(-) diff --git a/h5p.classes.php b/h5p.classes.php index 08f2a8e..72d84b6 100644 --- a/h5p.classes.php +++ b/h5p.classes.php @@ -1738,12 +1738,18 @@ class H5PCore { const DISABLE_COPYRIGHT = 8; const DISABLE_ABOUT = 16; + const DISPLAY_OPTION_FRAME = 'frame'; + const DISPLAY_OPTION_DOWNLOAD = 'export'; + const DISPLAY_OPTION_EMBED = 'embed'; + const DISPLAY_OPTION_COPYRIGHT = 'copyright'; + const DISPLAY_OPTION_ABOUT = 'icon'; + // Map flags to string public static $disable = array( - self::DISABLE_FRAME => 'frame', - self::DISABLE_DOWNLOAD => 'download', - self::DISABLE_EMBED => 'embed', - self::DISABLE_COPYRIGHT => 'copyright' + self::DISABLE_FRAME => self::DISPLAY_OPTION_FRAME, + self::DISABLE_DOWNLOAD => self::DISPLAY_OPTION_DOWNLOAD, + self::DISABLE_EMBED => self::DISPLAY_OPTION_EMBED, + self::DISABLE_COPYRIGHT => self::DISPLAY_OPTION_COPYRIGHT ); /** @@ -2473,17 +2479,17 @@ class H5PCore { */ public function getStorableDisplayOptions(&$sources, $current) { // Download - force setting it if always on or always off - $export = $this->h5pF->getOption('export', H5PDisplayOptionBehaviour::ALWAYS_SHOW); - if ($export == H5PDisplayOptionBehaviour::ALWAYS_SHOW || - $export == H5PDisplayOptionBehaviour::NEVER_SHOW) { - $sources['download'] = ($export == H5PDisplayOptionBehaviour::ALWAYS_SHOW); + $download = $this->h5pF->getOption(self::DISPLAY_OPTION_DOWNLOAD, H5PDisplayOptionBehaviour::ALWAYS_SHOW); + if ($download == H5PDisplayOptionBehaviour::ALWAYS_SHOW || + $download == H5PDisplayOptionBehaviour::NEVER_SHOW) { + $sources[self::DISPLAY_OPTION_DOWNLOAD] = ($download == H5PDisplayOptionBehaviour::ALWAYS_SHOW); } // Embed - force setting it if always on or always off - $embed = $this->h5pF->getOption('embed', H5PDisplayOptionBehaviour::ALWAYS_SHOW); + $embed = $this->h5pF->getOption(self::DISPLAY_OPTION_EMBED, H5PDisplayOptionBehaviour::ALWAYS_SHOW); if ($embed == H5PDisplayOptionBehaviour::ALWAYS_SHOW || $embed == H5PDisplayOptionBehaviour::NEVER_SHOW) { - $sources['embed'] = ($embed == H5PDisplayOptionBehaviour::ALWAYS_SHOW); + $sources[self::DISPLAY_OPTION_EMBED] = ($embed == H5PDisplayOptionBehaviour::ALWAYS_SHOW); } foreach (H5PCore::$disable as $bit => $option) { @@ -2508,34 +2514,37 @@ class H5PCore { $current_display_options = $disable === NULL ? [] : $this->getDisplayOptionsAsArray($disable); - if ($this->h5pF->getOption('frame', TRUE)) { - $display_options['frame'] = isset($current_display_options['showFrame']) ? $current_display_options['showFrame'] : TRUE; + if ($this->h5pF->getOption(self::DISPLAY_OPTION_FRAME, TRUE)) { + $display_options[self::DISPLAY_OPTION_FRAME] = + isset($current_display_options[self::DISPLAY_OPTION_FRAME]) ? + $current_display_options[self::DISPLAY_OPTION_FRAME] : + TRUE; // Download - $export = $this->h5pF->getOption('export', H5PDisplayOptionBehaviour::ALWAYS_SHOW); + $export = $this->h5pF->getOption(self::DISPLAY_OPTION_DOWNLOAD, H5PDisplayOptionBehaviour::ALWAYS_SHOW); if ($export == H5PDisplayOptionBehaviour::CONTROLLED_BY_AUTHOR_DEFAULT_ON || $export == H5PDisplayOptionBehaviour::CONTROLLED_BY_AUTHOR_DEFAULT_OFF) { - $display_options['download'] = - isset($current_display_options['showDownload']) ? - $current_display_options['showDownload'] : + $display_options[self::DISPLAY_OPTION_DOWNLOAD] = + isset($current_display_options[self::DISPLAY_OPTION_DOWNLOAD]) ? + $current_display_options[self::DISPLAY_OPTION_DOWNLOAD] : ($export == H5PDisplayOptionBehaviour::CONTROLLED_BY_AUTHOR_DEFAULT_ON); } // Embed - $embed = $this->h5pF->getOption('embed', H5PDisplayOptionBehaviour::ALWAYS_SHOW); + $embed = $this->h5pF->getOption(self::DISPLAY_OPTION_EMBED, H5PDisplayOptionBehaviour::ALWAYS_SHOW); if ($embed == H5PDisplayOptionBehaviour::CONTROLLED_BY_AUTHOR_DEFAULT_ON || $embed == H5PDisplayOptionBehaviour::CONTROLLED_BY_AUTHOR_DEFAULT_OFF) { - $display_options['embed'] = - isset($current_display_options['showEmbed']) ? - $current_display_options['showEmbed'] : + $display_options[self::DISPLAY_OPTION_EMBED] = + isset($current_display_options[self::DISPLAY_OPTION_EMBED]) ? + $current_display_options[self::DISPLAY_OPTION_EMBED] : ($embed == H5PDisplayOptionBehaviour::CONTROLLED_BY_AUTHOR_DEFAULT_ON); } // Copyright - if ($this->h5pF->getOption('copyright', TRUE)) { - $display_options['copyright'] = - isset($current_display_options['showCopyright']) ? - $current_display_options['showCopyright'] : + if ($this->h5pF->getOption(self::DISPLAY_OPTION_COPYRIGHT, TRUE)) { + $display_options[self::DISPLAY_OPTION_COPYRIGHT] = + isset($current_display_options[self::DISPLAY_OPTION_COPYRIGHT]) ? + $current_display_options[self::DISPLAY_OPTION_COPYRIGHT] : TRUE; } } @@ -2577,15 +2586,15 @@ class H5PCore { public function getDisplayOptionsForView($disable, $id) { $display_options = $this->getDisplayOptionsAsArray($disable); - if ($this->h5pF->getOption('frame', TRUE) == FALSE) { - $display_options['showFrame'] = false; + if ($this->h5pF->getOption(self::DISPLAY_OPTION_FRAME, TRUE) == FALSE) { + $display_options[self::DISPLAY_OPTION_FRAME] = false; } else { - $this->setDisplayOptionOverrides('export', H5PPermission::DOWNLOAD_H5P, $id, $display_options['showDownload']); - $this->setDisplayOptionOverrides('embed', H5PPermission::EMBED_H5P, $id, $display_options['showEmbed']); + $this->setDisplayOptionOverrides(self::DISPLAY_OPTION_DOWNLOAD, H5PPermission::DOWNLOAD_H5P, $id, $display_options[self::DISPLAY_OPTION_DOWNLOAD]); + $this->setDisplayOptionOverrides(self::DISPLAY_OPTION_EMBED, H5PPermission::EMBED_H5P, $id, $display_options[self::DISPLAY_OPTION_EMBED]); - if ($this->h5pF->getOption('copyright', TRUE) == FALSE) { - $display_options['showCopyright'] = false; + if ($this->h5pF->getOption(self::DISPLAY_OPTION_COPYRIGHT, TRUE) == FALSE) { + $display_options[self::DISPLAY_OPTION_COPYRIGHT] = false; } } @@ -2600,11 +2609,11 @@ class H5PCore { */ private function getDisplayOptionsAsArray($disable) { return array( - 'showFrame' => !($disable & H5PCore::DISABLE_FRAME), - 'showDownload' => !($disable & H5PCore::DISABLE_DOWNLOAD), - 'showEmbed' => !($disable & H5PCore::DISABLE_EMBED), - 'showCopyright' => !($disable & H5PCore::DISABLE_COPYRIGHT), - 'showAbout' => $this->h5pF->getOption('icon', TRUE), + self::DISPLAY_OPTION_FRAME => !($disable & H5PCore::DISABLE_FRAME), + self::DISPLAY_OPTION_DOWNLOAD => !($disable & H5PCore::DISABLE_DOWNLOAD), + self::DISPLAY_OPTION_EMBED => !($disable & H5PCore::DISABLE_EMBED), + self::DISPLAY_OPTION_COPYRIGHT => !($disable & H5PCore::DISABLE_COPYRIGHT), + self::DISPLAY_OPTION_ABOUT => $this->h5pF->getOption(self::DISPLAY_OPTION_ABOUT, TRUE), ); } diff --git a/js/h5p-action-bar.js b/js/h5p-action-bar.js index aba17e3..8e34eb7 100644 --- a/js/h5p-action-bar.js +++ b/js/h5p-action-bar.js @@ -44,17 +44,17 @@ H5P.ActionBar = (function ($, EventDispatcher) { }; // Register action bar buttons - if (displayOptions.showDownload) { + if (displayOptions.export) { // Add export button addActionButton('download', 'export'); } - if (displayOptions.showCopyright) { + if (displayOptions.copyright) { addActionButton('copyrights'); } - if (displayOptions.showEmbed) { + if (displayOptions.embed) { addActionButton('embed'); } - if (displayOptions.showAbout) { + if (displayOptions.icon) { // Add about H5P button icon H5P.jQuery('
    • ').appendTo($actions); hasActions = true; diff --git a/js/h5p.js b/js/h5p.js index 4af85e9..a080ebf 100644 --- a/js/h5p.js +++ b/js/h5p.js @@ -133,12 +133,12 @@ H5P.init = function (target) { * Create action bar */ var displayOptions = contentData.displayOptions; - if (displayOptions.showFrame) { + if (displayOptions.frame) { // Special handling of copyrights - if (displayOptions.showCopyright) { + if (displayOptions.copyright) { var copyrights = H5P.getCopyrights(instance, library.params, contentId); if (!copyrights) { - displayOptions.showCopyright = false; + displayOptions.copyright = false; } } @@ -163,7 +163,7 @@ H5P.init = function (target) { $actions.insertAfter($container); } - if (displayOptions.showFrame && actionBar.hasActions()) { + if (displayOptions.frame && actionBar.hasActions()) { $element.addClass('h5p-frame'); } else { From 1341768c90ae243e1dfef8cba31d9ee04656abc1 Mon Sep 17 00:00:00 2001 From: Paal Joergensen Date: Tue, 20 Dec 2016 14:37:31 +0100 Subject: [PATCH 9/9] Fixed problem with empty actionbar [HFP-277] --- js/h5p.js | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/js/h5p.js b/js/h5p.js index a080ebf..aa313b1 100644 --- a/js/h5p.js +++ b/js/h5p.js @@ -133,6 +133,7 @@ H5P.init = function (target) { * Create action bar */ var displayOptions = contentData.displayOptions; + var displayFrame = false; if (displayOptions.frame) { // Special handling of copyrights if (displayOptions.copyright) { @@ -160,15 +161,13 @@ H5P.init = function (target) { }); }); - $actions.insertAfter($container); + if (actionBar.hasActions()) { + displayFrame = true; + $actions.insertAfter($container); + } } - if (displayOptions.frame && actionBar.hasActions()) { - $element.addClass('h5p-frame'); - } - else { - $element.addClass('h5p-no-frame'); - } + $element.addClass(displayFrame ? 'h5p-frame' : 'h5p-no-frame'); // Keep track of when we started H5P.opened[contentId] = new Date();