From b11916d3e732c6e18d4b29163cacf20a570c9137 Mon Sep 17 00:00:00 2001 From: Marina Glancy Date: Fri, 18 Nov 2016 17:27:38 +0800 Subject: [PATCH 1/4] MDL-56586 blocks: allow theme to manage "Add a block" select --- .../tests/behat/addblockinactivity.feature | 3 + lib/blocklib.php | 74 +++++++++++++++++-- lib/navigationlib.php | 13 ++++ lib/outputlib.php | 2 +- theme/boost/config.php | 1 + .../behat/behat_theme_boost_behat_blocks.php | 11 +-- .../behat_theme_boost_behat_navigation.php | 52 +++++++++++++ theme/upgrade.txt | 2 + 8 files changed, 140 insertions(+), 18 deletions(-) diff --git a/blocks/activity_results/tests/behat/addblockinactivity.feature b/blocks/activity_results/tests/behat/addblockinactivity.feature index 2c62fb7e157..394234906f6 100644 --- a/blocks/activity_results/tests/behat/addblockinactivity.feature +++ b/blocks/activity_results/tests/behat/addblockinactivity.feature @@ -83,16 +83,19 @@ Scenario: Block should select current activity by default When I add the "Activity results" block And I configure the "Activity results" block Then the field "id_config_activitygradeitemid" matches value "Test assignment 1" + And I press "Cancel" And I follow "Course 1" And I follow "Test assignment 2" And I add the "Activity results" block And I configure the "Activity results" block And the field "id_config_activitygradeitemid" matches value "Test assignment 2" + And I press "Cancel" And I follow "Course 1" And I follow "Test assignment 3" And I add the "Activity results" block And I configure the "Activity results" block And the field "id_config_activitygradeitemid" matches value "Test assignment 3" + And I press "Cancel" And I follow "Course 1" And I follow "Test page name" And I add the "Activity results" block diff --git a/lib/blocklib.php b/lib/blocklib.php index 5a159c75d05..c592b1d9de2 100644 --- a/lib/blocklib.php +++ b/lib/blocklib.php @@ -42,6 +42,14 @@ define('BUI_CONTEXTS_ENTIRE_SITE', 2); define('BUI_CONTEXTS_CURRENT', 0); define('BUI_CONTEXTS_CURRENT_SUBS', 1); +// Position of "Add block" control, to be used in theme config as a value for $THEME->addblockposition: +// - default: as a fake block that is displayed in editing mode +// - flatnav: "Add block" item in the flat navigation drawer in editing mode +// - custom: none of the above, theme will take care of displaying the control. +define('BLOCK_ADDBLOCK_POSITION_DEFAULT', 0); +define('BLOCK_ADDBLOCK_POSITION_FLATNAV', 1); +define('BLOCK_ADDBLOCK_POSITION_CUSTOM', -1); + /** * Exception thrown when someone tried to do something with a block that does * not exist on a page. @@ -216,10 +224,12 @@ class block_manager { ($bi->instance_allow_multiple() || !$this->is_block_present($block->name)) && blocks_name_allowed_in_format($block->name, $pageformat) && $bi->user_can_addto($this->page)) { + $block->title = $bi->get_title(); $this->addableblocks[$block->name] = $block; } } + core_collator::asort_objects_by_property($this->addableblocks, 'title'); return $this->addableblocks; } @@ -1141,7 +1151,8 @@ class block_manager { $contents = $this->extracontent[$region]; } $contents = array_merge($contents, $this->create_block_contents($this->blockinstances[$region], $output, $region)); - if ($region == $this->defaultregion) { + if (($region == $this->defaultregion) && (!isset($this->page->theme->addblockposition) || + $this->page->theme->addblockposition == BLOCK_ADDBLOCK_POSITION_DEFAULT)) { $addblockui = block_add_block_ui($this->page, $output); if ($addblockui) { $contents[] = $addblockui; @@ -1286,8 +1297,10 @@ class block_manager { * @return boolean true if anything was done. False if not. */ public function process_url_add() { + global $CFG, $PAGE, $OUTPUT; + $blocktype = optional_param('bui_addblock', null, PARAM_PLUGIN); - if (!$blocktype) { + if ($blocktype === null) { return false; } @@ -1297,7 +1310,54 @@ class block_manager { throw new moodle_exception('nopermissions', '', $this->page->url->out(), get_string('addblock')); } - if (!array_key_exists($blocktype, $this->get_addable_blocks())) { + $addableblocks = $this->get_addable_blocks(); + + if ($blocktype === '') { + // Display add block selection. + $addpage = new moodle_page(); + $addpage->set_pagelayout('admin'); + $addpage->blocks->show_only_fake_blocks(true); + $addpage->set_course($this->page->course); + $addpage->set_context($this->page->context); + if ($this->page->cm) { + $addpage->set_cm($this->page->cm); + } + + $addpagebase = str_replace($CFG->wwwroot . '/', '/', $this->page->url->out_omit_querystring()); + $addpageparams = $this->page->url->params(); + $addpage->set_url($addpagebase, $addpageparams); + $addpage->set_block_actions_done(); + // At this point we are going to display the block selector, overwrite global $PAGE ready for this. + $PAGE = $addpage; + // Some functions use $OUTPUT so we need to replace that too. + $OUTPUT = $addpage->get_renderer('core'); + + $site = get_site(); + $straddblock = get_string('addblock'); + + $PAGE->navbar->add($straddblock); + $PAGE->set_title($straddblock); + $PAGE->set_heading($site->fullname); + echo $OUTPUT->header(); + echo $OUTPUT->heading($straddblock); + + if (!$addableblocks) { + echo $OUTPUT->box(get_string('noblockstoaddhere')); + echo $OUTPUT->container($OUTPUT->action_link($addpage->url, get_string('back')), 'm-x-3 m-b-1'); + } else { + foreach ($addableblocks as $blockname => $block) { + $url = new moodle_url($addpage->url, array('sesskey' => sesskey(), 'bui_addblock' => $blockname)); + echo $OUTPUT->container($OUTPUT->single_button($url, $block->title), 'm-x-3 m-b-1'); + } + echo $OUTPUT->container($OUTPUT->action_link($addpage->url, get_string('cancel')), 'm-x-3 m-b-1'); + } + + echo $OUTPUT->footer(); + // Make sure that nothing else happens after we have displayed this form. + exit; + } + + if (!array_key_exists($blocktype, $addableblocks)) { throw new moodle_exception('cannotaddthisblocktype', '', $this->page->url->out(), $blocktype); } @@ -1332,6 +1392,7 @@ class block_manager { if (!$confirmdelete) { $deletepage = new moodle_page(); $deletepage->set_pagelayout('admin'); + $deletepage->blocks->show_only_fake_blocks(true); $deletepage->set_course($this->page->course); $deletepage->set_context($this->page->context); if ($this->page->cm) { @@ -1452,6 +1513,7 @@ class block_manager { $editpage = new moodle_page(); $editpage->set_pagelayout('admin'); + $editpage->blocks->show_only_fake_blocks(true); $editpage->set_course($this->page->course); //$editpage->set_context($block->context); $editpage->set_context($this->page->context); @@ -2072,12 +2134,8 @@ function block_add_block_ui($page, $output) { $menu = array(); foreach ($missingblocks as $block) { - $blockobject = block_instance($block->name); - if ($blockobject !== false && $blockobject->user_can_addto($page)) { - $menu[$block->name] = $blockobject->get_title(); - } + $menu[$block->name] = $block->title; } - core_collator::asort($menu); $actionurl = new moodle_url($page->url, array('sesskey'=>sesskey())); $select = new single_select($actionurl, 'bui_addblock', $menu, null, array(''=>get_string('adddots')), 'add_block'); diff --git a/lib/navigationlib.php b/lib/navigationlib.php index a44205d1e3c..1517456005f 100644 --- a/lib/navigationlib.php +++ b/lib/navigationlib.php @@ -3764,6 +3764,19 @@ class flat_navigation extends navigation_node_collection { $flat->key = 'sitesettings'; $this->add($flat); } + + // Add-a-block in editing mode. + if (isset($this->page->theme->addblockposition) && + $this->page->theme->addblockposition == BLOCK_ADDBLOCK_POSITION_FLATNAV && + $PAGE->user_is_editing() && $PAGE->user_can_edit_blocks() && + $PAGE->blocks->get_addable_blocks()) { + $url = new moodle_url($PAGE->url, ['bui_addblock' => '', 'sesskey' => sesskey()]); + $addablock = navigation_node::create(get_string('addblock'), $url); + $flat = new flat_navigation_node($addablock, 0); + $flat->set_showdivider(true); + $flat->key = 'addblock'; + $this->add($flat); + } } } diff --git a/lib/outputlib.php b/lib/outputlib.php index 053432bf8dd..1f93bd253db 100644 --- a/lib/outputlib.php +++ b/lib/outputlib.php @@ -546,7 +546,7 @@ class theme_config { 'rendererfactory', 'csspostprocess', 'editor_sheets', 'rarrow', 'larrow', 'uarrow', 'darrow', 'hidefromselector', 'doctype', 'yuicssmodules', 'blockrtlmanipulations', 'lessfile', 'extralesscallback', 'lessvariablescallback', 'blockrendermethod', - 'scss', 'extrascsscallback', 'prescsscallback', 'csstreepostprocessor'); + 'scss', 'extrascsscallback', 'prescsscallback', 'csstreepostprocessor', 'addblockposition'); foreach ($config as $key=>$value) { if (in_array($key, $configurable)) { diff --git a/theme/boost/config.php b/theme/boost/config.php index d5794fc8ea5..fc0ae3751eb 100644 --- a/theme/boost/config.php +++ b/theme/boost/config.php @@ -152,3 +152,4 @@ $THEME->prescsscallback = 'theme_boost_get_pre_scss'; $THEME->yuicssmodules = array(); $THEME->rendererfactory = 'theme_overridden_renderer_factory'; $THEME->undeletableblocktypes = ''; +$THEME->addblockposition = BLOCK_ADDBLOCK_POSITION_FLATNAV; diff --git a/theme/boost/tests/behat/behat_theme_boost_behat_blocks.php b/theme/boost/tests/behat/behat_theme_boost_behat_blocks.php index 10716493fb1..319986d3cb3 100644 --- a/theme/boost/tests/behat/behat_theme_boost_behat_blocks.php +++ b/theme/boost/tests/behat/behat_theme_boost_behat_blocks.php @@ -38,15 +38,8 @@ require_once(__DIR__ . '/../../../../blocks/tests/behat/behat_blocks.php'); class behat_theme_boost_behat_blocks extends behat_blocks { public function i_add_the_block($blockname) { - $this->execute('behat_forms::i_set_the_field_to', - array("bui_addblock", $this->escape($blockname)) - ); - - // If we are running without javascript we need to submit the form. - if (!$this->running_javascript()) { - $this->execute('behat_general::i_click_on_in_the', - array("Go", "button", "Add a block", "block")); - } + $this->execute('behat_navigation::i_select_from_flat_navigation_drawer', get_string('addblock')); + $this->execute('behat_forms::press_button', $blockname); } public function i_open_the_blocks_action_menu($blockname) { diff --git a/theme/boost/tests/behat/behat_theme_boost_behat_navigation.php b/theme/boost/tests/behat/behat_theme_boost_behat_navigation.php index 97ed9d673de..a9469d1c3a6 100644 --- a/theme/boost/tests/behat/behat_theme_boost_behat_navigation.php +++ b/theme/boost/tests/behat/behat_theme_boost_behat_navigation.php @@ -76,4 +76,56 @@ class behat_theme_boost_behat_navigation extends behat_navigation { return $node; } + + /** + * Opens the flat navigation drawer if it is not already open + * + * @When /^I open flat navigation drawer$/ + * @throws ElementNotFoundException Thrown by behat_base::find + */ + public function i_open_flat_navigation_drawer() { + if (!$this->running_javascript()) { + // Navigation drawer is always open without JS. + return; + } + $xpath = "//button[contains(@data-action,'toggle-drawer')]"; + $node = $this->find('xpath', $xpath); + $expanded = $node->getAttribute('aria-expanded'); + if ($expanded === 'false') { + $node->click(); + $this->wait_for_pending_js(); + } + } + + /** + * Closes the flat navigation drawer if it is open (does nothing if JS disabled) + * + * @When /^I close flat navigation drawer$/ + * @throws ElementNotFoundException Thrown by behat_base::find + */ + public function i_close_flat_navigation_drawer() { + if (!$this->running_javascript()) { + // Navigation drawer can not be closed without JS. + return; + } + $xpath = "//button[contains(@data-action,'toggle-drawer')]"; + $node = $this->find('xpath', $xpath); + $expanded = $node->getAttribute('aria-expanded'); + if ($expanded === 'true') { + $node->click(); + $this->wait_for_pending_js(); + } + } + + /** + * Clicks link with specified id|title|alt|text in the flat navigation drawer. + * + * @When /^I select "(?P(?:[^"]|\\")*)" from flat navigation drawer$/ + * @throws ElementNotFoundException Thrown by behat_base::find + * @param string $link + */ + public function i_select_from_flat_navigation_drawer($link) { + $this->i_open_flat_navigation_drawer(); + $this->execute('behat_general::i_click_on_in_the', [$link, 'link', '#nav-drawer', 'css_element']); + } } diff --git a/theme/upgrade.txt b/theme/upgrade.txt index 7efedcb3dbc..6387ad8a199 100644 --- a/theme/upgrade.txt +++ b/theme/upgrade.txt @@ -57,6 +57,8 @@ information provided here is intended especially for theme designer. * CLI svgtool.php has moved from theme/base/cli to admin/cli and paths should be relative to the new location. * mod_chat will now display the 'course theme' option for all themes (previously it was only displayed on bootstrap2 based themes). +* Theme can choose how to display "Add a block" control in $THEME->addblockposition, default value is + BLOCK_ADDBLOCK_POSITION_DEFAULT that displays it as a fake block in editing mode. === 3.1 === From b1e5624ad9913b0be5c4766c76bfd6b297e44a0e Mon Sep 17 00:00:00 2001 From: Marina Glancy Date: Mon, 21 Nov 2016 15:53:00 +0800 Subject: [PATCH 2/4] MDL-56586 blocks: Fix mess with links to permissions problem 1: links to blocks permissions changes were missing in boost problem 2: when modifying link from dashboard user is returned to profile ('returnurl' passed but not respected) problem 3: link to check permissions missing from category management page fix: instead of respecting both 'return' and 'returnurl' parameter use only 'returnurl' fix: use 'popup' layout when modifying permissions for a block --- admin/roles/assign.php | 39 +++++++----------- admin/roles/check.php | 31 ++++++++------ admin/roles/permissions.php | 19 +++++++-- course/classes/management/helper.php | 17 ++++++-- course/tests/management_helper_test.php | 3 ++ lib/blocklib.php | 55 +++++++++++++------------ 6 files changed, 96 insertions(+), 68 deletions(-) diff --git a/admin/roles/assign.php b/admin/roles/assign.php index f5358660441..889a2d111dc 100644 --- a/admin/roles/assign.php +++ b/admin/roles/assign.php @@ -29,7 +29,7 @@ define("MAX_USERS_TO_LIST_PER_ROLE", 10); $contextid = required_param('contextid', PARAM_INT); $roleid = optional_param('roleid', 0, PARAM_INT); -$returnto = optional_param('return', null, PARAM_ALPHANUMEXT); +$returnurl = optional_param('returnurl', null, PARAM_LOCALURL); list($context, $course, $cm) = get_context_info_array($contextid); @@ -53,7 +53,13 @@ if ($course) { // Security. require_login($course, false, $cm); require_capability('moodle/role:assign', $context); -$PAGE->set_url($url); + +navigation_node::override_active_url($url); +$pageurl = new moodle_url($url); +if ($returnurl) { + $pageurl->param('returnurl', $returnurl); +} +$PAGE->set_url($pageurl); $PAGE->set_context($context); $contextname = $context->get_context_name(); @@ -141,6 +147,10 @@ if (!empty($user) && ($user->id != $USER->id)) { } $PAGE->set_pagelayout('admin'); +if ($context->contextlevel == CONTEXT_BLOCK) { + // Do not show blocks when changing block's settings, it is confusing. + $PAGE->blocks->show_only_fake_blocks(true); +} $PAGE->set_title($title); switch ($context->contextlevel) { @@ -186,9 +196,6 @@ if ($roleid) { // Print the form. $assignurl = new moodle_url($PAGE->url, array('roleid'=>$roleid)); - if ($returnto !== null) { - $assignurl->param('return', $returnto); - } ?>
@@ -235,18 +242,10 @@ if ($roleid) { // Print a form to swap roles, and a link back to the all roles list. echo '
'; - $newroleurl = new moodle_url($PAGE->url); - if ($returnto !== null) { - $newroleurl->param('return', $returnto); - } - $select = new single_select($newroleurl, 'roleid', $nameswithcounts, $roleid, null); + $select = new single_select($PAGE->url, 'roleid', $nameswithcounts, $roleid, null); $select->label = get_string('assignanotherrole', 'core_role'); echo $OUTPUT->render($select); - $backurl = new moodle_url('/admin/roles/assign.php', array('contextid' => $contextid)); - if ($returnto !== null) { - $backurl->param('return', $returnto); - } - echo '

' . get_string('backtoallroles', 'core_role') . '

'; + echo '

' . get_string('backtoallroles', 'core_role') . '

'; echo '
'; } else if (empty($assignableroles)) { @@ -284,9 +283,6 @@ if ($roleid) { } } else if ($assigncounts[$roleid] > MAX_USERS_TO_LIST_PER_ROLE) { $assignurl = new moodle_url($PAGE->url, array('roleid'=>$roleid)); - if ($returnto !== null) { - $assignurl->param('return', $returnto); - } $roleholdernames[$roleid] = ''.$strmorethanmax.''; } else { $roleholdernames[$roleid] = ''; @@ -307,9 +303,6 @@ if ($roleid) { foreach ($assignableroles as $roleid => $rolename) { $description = format_string($DB->get_field('role', 'description', array('id'=>$roleid))); $assignurl = new moodle_url($PAGE->url, array('roleid'=>$roleid)); - if ($returnto !== null) { - $assignurl->param('return', $returnto); - } $row = array(''.$rolename.'', $description, $assigncounts[$roleid]); if ($showroleholders) { @@ -322,8 +315,8 @@ if ($roleid) { if ($context->contextlevel > CONTEXT_USER) { - if ($context->contextlevel === CONTEXT_COURSECAT && $returnto === 'management') { - $url = new moodle_url('/course/management.php', array('categoryid' => $context->instanceid)); + if ($returnurl) { + $url = new moodle_url($returnurl); } else { $url = $context->get_url(); } diff --git a/admin/roles/check.php b/admin/roles/check.php index 0f403080219..10e173ef13e 100644 --- a/admin/roles/check.php +++ b/admin/roles/check.php @@ -25,6 +25,7 @@ require_once(__DIR__ . '/../../config.php'); $contextid = required_param('contextid', PARAM_INT); +$returnurl = optional_param('returnurl', null, PARAM_LOCALURL); list($context, $course, $cm) = get_context_info_array($contextid); @@ -49,7 +50,13 @@ require_login($course, false, $cm); if (!has_any_capability(array('moodle/role:assign', 'moodle/role:safeoverride', 'moodle/role:override', 'moodle/role:manage'), $context)) { print_error('nopermissions', 'error', '', get_string('checkpermissions', 'core_role')); } -$PAGE->set_url($url); + +navigation_node::override_active_url($url); +$pageurl = new moodle_url($url); +if ($returnurl) { + $pageurl->param('returnurl', $returnurl); +} +$PAGE->set_url($pageurl); if ($context->contextlevel == CONTEXT_USER and $USER->id != $context->instanceid) { $PAGE->navbar->includesettingsbase = true; @@ -75,6 +82,10 @@ $userselector->set_rows(20); $title = get_string('checkpermissionsin', 'core_role', $contextname); $PAGE->set_pagelayout('admin'); +if ($context->contextlevel == CONTEXT_BLOCK) { + // Do not show blocks when changing block's settings, it is confusing. + $PAGE->blocks->show_only_fake_blocks(true); +} $PAGE->set_title($title); switch ($context->contextlevel) { @@ -156,16 +167,7 @@ if (!is_null($reportuser)) { // Show UI for choosing a user to report on. echo $OUTPUT->box_start('generalbox boxwidthnormal boxaligncenter', 'chooseuser'); -echo ''; - -// Hidden fields. -echo ''; -if (!empty($user->id)) { - echo ''; -} -if ($isfrontpage) { - echo ''; -} +echo ''; // User selector. echo $OUTPUT->heading('', 3); @@ -180,7 +182,12 @@ echo $OUTPUT->box_end(); // Appropriate back link. if ($context->contextlevel > CONTEXT_USER) { echo html_writer::start_tag('div', array('class'=>'backlink')); - echo html_writer::tag('a', get_string('backto', '', $contextname), array('href'=>$context->get_url())); + if ($returnurl) { + $backurl = new moodle_url($returnurl); + } else { + $backurl = $context->get_url(); + } + echo html_writer::link($backurl, get_string('backto', '', $contextname)); echo html_writer::end_tag('div'); } diff --git a/admin/roles/permissions.php b/admin/roles/permissions.php index c83f819e8cb..89668dc53c5 100644 --- a/admin/roles/permissions.php +++ b/admin/roles/permissions.php @@ -33,7 +33,7 @@ $prevent = optional_param('prevent', 0, PARAM_BOOL); $allow = optional_param('allow', 0, PARAM_BOOL); $unprohibit = optional_param('unprohibit', 0, PARAM_BOOL); $prohibit = optional_param('prohibit', 0, PARAM_BOOL); -$return = optional_param('return', null, PARAM_ALPHANUMEXT); +$returnurl = optional_param('returnurl', null, PARAM_LOCALURL); list($context, $course, $cm) = get_context_info_array($contextid); @@ -56,7 +56,13 @@ if ($course) { // Security first. require_login($course, false, $cm); require_capability('moodle/role:review', $context); -$PAGE->set_url($url); + +navigation_node::override_active_url($url); +$pageurl = new moodle_url($url); +if ($returnurl) { + $pageurl->param('returnurl', $returnurl); +} +$PAGE->set_url($pageurl); if ($context->contextlevel == CONTEXT_USER and $USER->id != $context->instanceid) { $PAGE->navbar->includesettingsbase = true; @@ -85,6 +91,11 @@ $straction = get_string('permissions', 'core_role'); // Used by tabs.php. $currenttab = 'permissions'; $PAGE->set_pagelayout('admin'); +if ($context->contextlevel == CONTEXT_BLOCK) { + // Do not show blocks when changing block's settings, it is confusing. + $PAGE->blocks->show_only_fake_blocks(true); +} + $PAGE->set_title($title); switch ($context->contextlevel) { case CONTEXT_SYSTEM: @@ -219,8 +230,8 @@ echo $OUTPUT->box_end(); if ($context->contextlevel > CONTEXT_USER) { - if ($context->contextlevel === CONTEXT_COURSECAT && $return === 'management') { - $url = new moodle_url('/course/management.php', array('categoryid' => $context->instanceid)); + if ($returnurl) { + $url = new moodle_url($returnurl); } else { $url = $context->get_url(); } diff --git a/course/classes/management/helper.php b/course/classes/management/helper.php index b553923a969..94b6638d826 100644 --- a/course/classes/management/helper.php +++ b/course/classes/management/helper.php @@ -168,6 +168,7 @@ class helper { * @return array */ public static function get_category_listitem_actions(\coursecat $category) { + global $PAGE; $baseurl = new \moodle_url('/course/management.php', array('categoryid' => $category->id, 'sesskey' => \sesskey())); $actions = array(); // Edit. @@ -249,11 +250,11 @@ class helper { ); } - // Roles. + // Assign roles. if ($category->can_review_roles()) { $actions['assignroles'] = array( 'url' => new \moodle_url('/admin/roles/assign.php', array('contextid' => $category->get_context()->id, - 'return' => 'management')), + 'returnurl' => $PAGE->url->out_as_local_url(false))), 'icon' => new \pix_icon('t/assignroles', new \lang_string('assignroles', 'role')), 'string' => new \lang_string('assignroles', 'role') ); @@ -263,12 +264,22 @@ class helper { if ($category->can_review_permissions()) { $actions['permissions'] = array( 'url' => new \moodle_url('/admin/roles/permissions.php', array('contextid' => $category->get_context()->id, - 'return' => 'management')), + 'returnurl' => $PAGE->url->out_as_local_url(false))), 'icon' => new \pix_icon('i/permissions', new \lang_string('permissions', 'role')), 'string' => new \lang_string('permissions', 'role') ); } + // Check permissions. + if ($category->can_review_permissions()) { + $actions['checkroles'] = array( + 'url' => new \moodle_url('/admin/roles/check.php', array('contextid' => $category->get_context()->id, + 'returnurl' => $PAGE->url->out_as_local_url(false))), + 'icon' => new \pix_icon('i/checkpermissions', new \lang_string('checkpermissions', 'role')), + 'string' => new \lang_string('checkpermissions', 'role') + ); + } + // Cohorts. if ($category->can_review_cohorts()) { $actions['cohorts'] = array( diff --git a/course/tests/management_helper_test.php b/course/tests/management_helper_test.php index ee9a952035a..6d1708b6896 100644 --- a/course/tests/management_helper_test.php +++ b/course/tests/management_helper_test.php @@ -1088,8 +1088,11 @@ class core_course_management_helper_test extends advanced_testcase { * Tests the fetching of actions for a category. */ public function test_get_category_listitem_actions() { + global $PAGE; $this->resetAfterTest(true); + $PAGE->set_url(new moodle_url('/course/management.php')); + $generator = $this->getDataGenerator(); $category = $generator->create_category(); $context = context_system::instance(); diff --git a/lib/blocklib.php b/lib/blocklib.php index c592b1d9de2..6dd989201e2 100644 --- a/lib/blocklib.php +++ b/lib/blocklib.php @@ -1221,36 +1221,39 @@ class block_manager { $controls[] = new action_menu_link_secondary($url, $icon, $str, $attributes); } - // Display either "Assign roles" or "Permissions" or "Change permissions" icon (whichever first is available). - $rolesurl = null; - + // Assign roles. if (get_assignable_roles($block->context, ROLENAME_SHORT)) { - $rolesurl = new moodle_url('/admin/roles/assign.php', array('contextid' => $block->context->id)); + $rolesurl = new moodle_url('/admin/roles/assign.php', array('contextid' => $block->context->id, + 'returnurl' => $this->page->url->out_as_local_url())); $str = new lang_string('assignrolesinblock', 'block', $blocktitle); - $icon = 'i/assignroles'; - } else if (has_capability('moodle/role:review', $block->context) or get_overridable_roles($block->context)) { - $rolesurl = new moodle_url('/admin/roles/permissions.php', array('contextid' => $block->context->id)); - $str = get_string('permissions', 'role'); - $icon = 'i/permissions'; - } else if (has_any_capability(array('moodle/role:safeoverride', 'moodle/role:override', 'moodle/role:assign'), $block->context)) { - $rolesurl = new moodle_url('/admin/roles/check.php', array('contextid' => $block->context->id)); - $str = get_string('checkpermissions', 'role'); - $icon = 'i/checkpermissions'; - } - - if ($rolesurl) { - // TODO: please note it is sloppy to pass urls through page parameters!! - // it is shortened because some web servers (e.g. IIS by default) give - // a 'security' error if you try to pass a full URL as a GET parameter in another URL. - $return = $this->page->url->out(false); - $return = str_replace($CFG->wwwroot . '/', '', $return); - $rolesurl->param('returnurl', $return); - $controls[] = new action_menu_link_secondary( $rolesurl, - new pix_icon($icon, $str, 'moodle', array('class' => 'iconsmall', 'title' => '')), - $str, - array('class' => 'editing_roles') + new pix_icon('i/assignroles', $str, 'moodle', array('class' => 'iconsmall', 'title' => '')), + $str, array('class' => 'editing_assignroles') + ); + } + + // Permissions. + if (has_capability('moodle/role:review', $block->context) or get_overridable_roles($block->context)) { + $rolesurl = new moodle_url('/admin/roles/permissions.php', array('contextid' => $block->context->id, + 'returnurl' => $this->page->url->out_as_local_url())); + $str = get_string('permissions', 'role'); + $controls[] = new action_menu_link_secondary( + $rolesurl, + new pix_icon('i/permissions', $str, 'moodle', array('class' => 'iconsmall', 'title' => '')), + $str, array('class' => 'editing_permissions') + ); + } + + // Change permissions. + if (has_any_capability(array('moodle/role:safeoverride', 'moodle/role:override', 'moodle/role:assign'), $block->context)) { + $rolesurl = new moodle_url('/admin/roles/check.php', array('contextid' => $block->context->id, + 'returnurl' => $this->page->url->out_as_local_url())); + $str = get_string('checkpermissions', 'role'); + $controls[] = new action_menu_link_secondary( + $rolesurl, + new pix_icon('i/checkpermissions', $str, 'moodle', array('class' => 'iconsmall', 'title' => '')), + $str, array('class' => 'editing_checkroles') ); } From be2247fb24e1fe86e689d371d85d37ed7a001131 Mon Sep 17 00:00:00 2001 From: Damyon Wiese Date: Wed, 23 Nov 2016 13:54:41 +0800 Subject: [PATCH 3/4] MDL-56586 blocks: Do the addblock in a modal --- lib/amd/build/addblockmodal.min.js | 1 + lib/amd/build/modal_cancel.min.js | 1 + lib/amd/build/modal_factory.min.js | 2 +- lib/amd/src/addblockmodal.js | 64 +++++++++++++++ lib/amd/src/modal_cancel.js | 77 +++++++++++++++++++ lib/amd/src/modal_factory.js | 10 ++- lib/navigationlib.php | 8 +- lib/templates/add_block_body.mustache | 37 +++++++++ lib/templates/modal_cancel.mustache | 45 +++++++++++ lib/templates/modal_confirm.mustache | 2 +- lib/templates/modal_save_cancel.mustache | 2 +- .../boost/templates/flat_navigation.mustache | 4 +- .../behat/behat_theme_boost_behat_blocks.php | 7 +- 13 files changed, 250 insertions(+), 10 deletions(-) create mode 100644 lib/amd/build/addblockmodal.min.js create mode 100644 lib/amd/build/modal_cancel.min.js create mode 100644 lib/amd/src/addblockmodal.js create mode 100644 lib/amd/src/modal_cancel.js create mode 100644 lib/templates/add_block_body.mustache create mode 100644 lib/templates/modal_cancel.mustache diff --git a/lib/amd/build/addblockmodal.min.js b/lib/amd/build/addblockmodal.min.js new file mode 100644 index 00000000000..7cf5cb8a2be --- /dev/null +++ b/lib/amd/build/addblockmodal.min.js @@ -0,0 +1 @@ +define(["jquery","core/modal_factory","core/templates","core/str","core/notification"],function(a,b,c,d,e){return{init:function(f){var g=a("[data-key=addblock]");d.get_string("addblock").done(function(e){var h=[];a.each(f.blocks,function(a,b){h[h.length]={key:"pluginname",component:"block_"+b}});var i=[];d.get_strings(h).done(function(d){a.each(d,function(a,b){i[i.length]={name:f.blocks[a],title:b}}),f.blocks=i,b.create({title:e,body:c.render("core/add_block_body",f),type:"CANCEL"},g)})}).fail(e.exception)}}}); \ No newline at end of file diff --git a/lib/amd/build/modal_cancel.min.js b/lib/amd/build/modal_cancel.min.js new file mode 100644 index 00000000000..b74ca27c031 --- /dev/null +++ b/lib/amd/build/modal_cancel.min.js @@ -0,0 +1 @@ +define(["jquery","core/notification","core/custom_interaction_events","core/modal","core/modal_events"],function(a,b,c,d,e){var f={CANCEL_BUTTON:'[data-action="cancel"]'},g=function(a){d.call(this,a),this.getFooter().find(f.CANCEL_BUTTON).length||b.exception({message:"No cancel button found"})};return g.prototype=Object.create(d.prototype),g.prototype.constructor=g,g.prototype.setFooter=function(){b.exception({message:"Can not change the footer of a cancel modal"})},g.prototype.registerEventListeners=function(){d.prototype.registerEventListeners.call(this),this.getModal().on(c.events.activate,f.CANCEL_BUTTON,function(b,c){var d=a.Event(e.cancel);this.getRoot().trigger(d,this),d.isDefaultPrevented()||(this.hide(),c.originalEvent.preventDefault())}.bind(this))},g}); \ No newline at end of file diff --git a/lib/amd/build/modal_factory.min.js b/lib/amd/build/modal_factory.min.js index 6c9d80af856..fb5a544cc95 100644 --- a/lib/amd/build/modal_factory.min.js +++ b/lib/amd/build/modal_factory.min.js @@ -1 +1 @@ -define(["jquery","core/modal_events","core/modal","core/modal_save_cancel","core/modal_confirm","core/templates","core/notification","core/custom_interaction_events"],function(a,b,c,d,e,f,g,h){var i={DEFAULT:"core/modal",SAVE_CANCEL:"core/modal_save_cancel",CONFIRM:"core/modal_confirm"},j={DEFAULT:c,SAVE_CANCEL:d,CONFIRM:e},k={DEFAULT:"DEFAULT",SAVE_CANCEL:"SAVE_CANCEL",CONFIRM:"CONFIRM"},l=function(a,c){"undefined"!=typeof c&&(h.define(c,[h.events.activate]),c.on(h.events.activate,function(){a.show()}),a.getRoot().on(b.hidden,function(){c.focus()}))},m=function(b,c,d){c=a(c);var e=j[b],f=new e(c);return l(f,d),f},n=function(b,c){var d=i[b];return f.render(d,{}).then(function(d){var e=a(d);return m(b,e,c)}).fail(g.exception)},o=function(a,b){var c=a.type||k.DEFAULT,d=!!a.large;return k[c]||(c=k.DEFAULT),n(c,b).then(function(b){return"undefined"!=typeof a.title&&b.setTitle(a.title),"undefined"!=typeof a.body&&b.setBody(a.body),"undefined"!=typeof a.footer&&b.setFooter(a.footer),d&&b.setLarge(),b})};return{create:o,types:k}}); \ No newline at end of file +define(["jquery","core/modal_events","core/modal","core/modal_save_cancel","core/modal_confirm","core/modal_cancel","core/templates","core/notification","core/custom_interaction_events"],function(a,b,c,d,e,f,g,h,i){var j={DEFAULT:"core/modal",SAVE_CANCEL:"core/modal_save_cancel",CONFIRM:"core/modal_confirm",CANCEL:"core/modal_cancel"},k={DEFAULT:c,SAVE_CANCEL:d,CONFIRM:e,CANCEL:f},l={DEFAULT:"DEFAULT",SAVE_CANCEL:"SAVE_CANCEL",CONFIRM:"CONFIRM",CANCEL:"CANCEL"},m=function(a,c){"undefined"!=typeof c&&(i.define(c,[i.events.activate]),c.on(i.events.activate,function(b,c){a.show(),c.originalEvent.preventDefault()}),a.getRoot().on(b.hidden,function(){c.focus()}))},n=function(b,c,d){c=a(c);var e=k[b],f=new e(c);return m(f,d),f},o=function(b,c){var d=j[b];return g.render(d,{}).then(function(d){var e=a(d);return n(b,e,c)}).fail(h.exception)},p=function(a,b){var c=a.type||l.DEFAULT,d=!!a.large;return l[c]||(c=l.DEFAULT),o(c,b).then(function(b){return"undefined"!=typeof a.title&&b.setTitle(a.title),"undefined"!=typeof a.body&&b.setBody(a.body),"undefined"!=typeof a.footer&&b.setFooter(a.footer),d&&b.setLarge(),b})};return{create:p,types:l}}); \ No newline at end of file diff --git a/lib/amd/src/addblockmodal.js b/lib/amd/src/addblockmodal.js new file mode 100644 index 00000000000..5da0bdec872 --- /dev/null +++ b/lib/amd/src/addblockmodal.js @@ -0,0 +1,64 @@ +// This file is part of Moodle - http://moodle.org/ +// +// Moodle is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Moodle is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Moodle. If not, see . + +/** + * Show an add block modal instead of doing it on a separate page. + * + * @module core/addblockmodal + * @class addblockmodal + * @package core + * @copyright 2016 Damyon Wiese + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +define(['jquery', 'core/modal_factory', 'core/templates', 'core/str', 'core/notification'], + function($, ModalFactory, Templates, Str, Notification) { + + + return /** @alias module:core/addblockmodal */ { + /** + * Global init function for this module. + * + * @method init + * @param {Object} context The template context for rendering this modal body. + */ + init: function(context) { + var addblocklink = $('[data-key=addblock]'); + + // We need the fetch the names of the blocks. It was too much to send in the page. + Str.get_string('addblock').done(function(title) { + + var titlerequests = []; + $.each(context.blocks, function(index, key) { + titlerequests[titlerequests.length] = {key: 'pluginname', component: 'block_' + key}; + }); + + var blocks = []; + Str.get_strings(titlerequests).done(function(titles) { + $.each(titles, function(index, title) { + blocks[blocks.length] = {name: context.blocks[index], title: title}; + }); + + context.blocks = blocks; + + ModalFactory.create({ + title: title, + body: Templates.render('core/add_block_body', context), + type: 'CANCEL', + }, addblocklink); + }); + }).fail(Notification.exception); + } + }; +}); diff --git a/lib/amd/src/modal_cancel.js b/lib/amd/src/modal_cancel.js new file mode 100644 index 00000000000..2e182c1e2d2 --- /dev/null +++ b/lib/amd/src/modal_cancel.js @@ -0,0 +1,77 @@ +// This file is part of Moodle - http://moodle.org/ +// +// Moodle is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Moodle is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Moodle. If not, see . + +/** + * Contain the logic for the cancel modal. + * + * @module core/modal_cancel + * @class modal_cancel + * @package core + * @copyright 2016 Ryan Wyllie + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +define(['jquery', 'core/notification', 'core/custom_interaction_events', 'core/modal', 'core/modal_events'], + function($, Notification, CustomEvents, Modal, ModalEvents) { + + var SELECTORS = { + CANCEL_BUTTON: '[data-action="cancel"]', + }; + + /** + * Constructor for the Modal. + * + * @param {object} root The root jQuery element for the modal + */ + var ModalCancel = function(root) { + Modal.call(this, root); + + if (!this.getFooter().find(SELECTORS.CANCEL_BUTTON).length) { + Notification.exception({message: 'No cancel button found'}); + } + }; + + ModalCancel.prototype = Object.create(Modal.prototype); + ModalCancel.prototype.constructor = ModalCancel; + + /** + * Override parent implementation to prevent changing the footer content. + */ + ModalCancel.prototype.setFooter = function() { + Notification.exception({message: 'Can not change the footer of a cancel modal'}); + return; + }; + + /** + * Set up all of the event handling for the modal. + * + * @method registerEventListeners + */ + ModalCancel.prototype.registerEventListeners = function() { + // Apply parent event listeners. + Modal.prototype.registerEventListeners.call(this); + + this.getModal().on(CustomEvents.events.activate, SELECTORS.CANCEL_BUTTON, function(e, data) { + var cancelEvent = $.Event(ModalEvents.cancel); + this.getRoot().trigger(cancelEvent, this); + + if (!cancelEvent.isDefaultPrevented()) { + this.hide(); + data.originalEvent.preventDefault(); + } + }.bind(this)); + }; + + return ModalCancel; +}); diff --git a/lib/amd/src/modal_factory.js b/lib/amd/src/modal_factory.js index 3622573904c..ffcb16dedfc 100644 --- a/lib/amd/src/modal_factory.js +++ b/lib/amd/src/modal_factory.js @@ -22,15 +22,16 @@ * @copyright 2016 Ryan Wyllie * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ -define(['jquery', 'core/modal_events', 'core/modal', 'core/modal_save_cancel', 'core/modal_confirm', +define(['jquery', 'core/modal_events', 'core/modal', 'core/modal_save_cancel', 'core/modal_confirm', 'core/modal_cancel', 'core/templates', 'core/notification', 'core/custom_interaction_events'], - function($, ModalEvents, Modal, ModalSaveCancel, ModalConfirm, Templates, Notification, CustomEvents) { + function($, ModalEvents, Modal, ModalSaveCancel, ModalConfirm, ModalCancel, Templates, Notification, CustomEvents) { // The templates for each type of modal. var TEMPLATES = { DEFAULT: 'core/modal', SAVE_CANCEL: 'core/modal_save_cancel', CONFIRM: 'core/modal_confirm', + CANCEL: 'core/modal_cancel', }; // The JS classes for each type of modal. @@ -38,6 +39,7 @@ define(['jquery', 'core/modal_events', 'core/modal', 'core/modal_save_cancel', ' DEFAULT: Modal, SAVE_CANCEL: ModalSaveCancel, CONFIRM: ModalConfirm, + CANCEL: ModalCancel, }; // The available types of modals. @@ -45,6 +47,7 @@ define(['jquery', 'core/modal_events', 'core/modal', 'core/modal_save_cancel', ' DEFAULT: 'DEFAULT', SAVE_CANCEL: 'SAVE_CANCEL', CONFIRM: 'CONFIRM', + CANCEL: 'CANCEL', }; /** @@ -58,8 +61,9 @@ define(['jquery', 'core/modal_events', 'core/modal', 'core/modal_save_cancel', ' var setUpTrigger = function(modal, triggerElement) { if (typeof triggerElement != 'undefined') { CustomEvents.define(triggerElement, [CustomEvents.events.activate]); - triggerElement.on(CustomEvents.events.activate, function() { + triggerElement.on(CustomEvents.events.activate, function(e, data) { modal.show(); + data.originalEvent.preventDefault(); }); modal.getRoot().on(ModalEvents.hidden, function() { diff --git a/lib/navigationlib.php b/lib/navigationlib.php index 1517456005f..3cedc3a30e6 100644 --- a/lib/navigationlib.php +++ b/lib/navigationlib.php @@ -3769,13 +3769,19 @@ class flat_navigation extends navigation_node_collection { if (isset($this->page->theme->addblockposition) && $this->page->theme->addblockposition == BLOCK_ADDBLOCK_POSITION_FLATNAV && $PAGE->user_is_editing() && $PAGE->user_can_edit_blocks() && - $PAGE->blocks->get_addable_blocks()) { + ($addable = $PAGE->blocks->get_addable_blocks())) { $url = new moodle_url($PAGE->url, ['bui_addblock' => '', 'sesskey' => sesskey()]); $addablock = navigation_node::create(get_string('addblock'), $url); $flat = new flat_navigation_node($addablock, 0); $flat->set_showdivider(true); $flat->key = 'addblock'; $this->add($flat); + $blocks = []; + foreach ($addable as $block) { + $blocks[] = $block->name; + } + $params = array('blocks' => $blocks, 'url' => '?' . $url->get_query_string(false)); + $PAGE->requires->js_call_amd('core/addblockmodal', 'init', array($params)); } } diff --git a/lib/templates/add_block_body.mustache b/lib/templates/add_block_body.mustache new file mode 100644 index 00000000000..21dbe125c76 --- /dev/null +++ b/lib/templates/add_block_body.mustache @@ -0,0 +1,37 @@ +{{! + This file is part of Moodle - http://moodle.org/ + + Moodle is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + Moodle is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with Moodle. If not, see . +}} +{{! + @template core/add_block_body + + Template for the body of the add block modal. + + Context variables required for this template: + * blocks - list of blocks containing name and title + * url - base url for links back to same page. + + Example context (json): + { + "blocks" : [ { "name": "test", "title": "Test block" } ], + "url" : "?a=b" + } + +}} +
+{{#blocks}} + {{title}} +{{/blocks}} +
diff --git a/lib/templates/modal_cancel.mustache b/lib/templates/modal_cancel.mustache new file mode 100644 index 00000000000..14884c0c5fb --- /dev/null +++ b/lib/templates/modal_cancel.mustache @@ -0,0 +1,45 @@ +{{! + This file is part of Moodle - http://moodle.org/ + + Moodle is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + Moodle is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with Moodle. If not, see . +}} +{{! + @template core/modal_save_cancel + + Moodle modal template with one cancel button. + + The purpose of this template is to render a modal. + + Classes required for JS: + * none + + Data attributes required for JS: + * none + + Context variables required for this template: + * title A cleaned string (use clean_text()) to display. + * body HTML content for the boday + + Example context (json): + { + "title": "Example cancel modal", + "body": "Some example content for the body" + } +}} + +{{< core/modal }} + {{$footer}} + + {{/footer}} +{{/ core/modal }} diff --git a/lib/templates/modal_confirm.mustache b/lib/templates/modal_confirm.mustache index f2398860ec0..549a3c657a9 100644 --- a/lib/templates/modal_confirm.mustache +++ b/lib/templates/modal_confirm.mustache @@ -40,6 +40,6 @@ {{$body}}{{#str}} areyousure {{/str}}{{/body}} {{$footer}} - + {{/footer}} {{/ core/modal }} diff --git a/lib/templates/modal_save_cancel.mustache b/lib/templates/modal_save_cancel.mustache index f58e402f6cd..4e6326069f4 100644 --- a/lib/templates/modal_save_cancel.mustache +++ b/lib/templates/modal_save_cancel.mustache @@ -41,6 +41,6 @@ {{< core/modal }} {{$footer}} - + {{/footer}} {{/ core/modal }} diff --git a/theme/boost/templates/flat_navigation.mustache b/theme/boost/templates/flat_navigation.mustache index fa3b35ab26e..e1aab0208f9 100644 --- a/theme/boost/templates/flat_navigation.mustache +++ b/theme/boost/templates/flat_navigation.mustache @@ -62,7 +62,7 @@