MDL-72952 reportbuilder: UX behaviour improvements in audiences

- Show a toast notification when saving an audience
- Add form change checker when adding a new audience to prevent user from navigating away if it is not saved
- Remove expand/collapse animation in audience sidebar to be consistent with editor

Co-authored-By: Paul Holden <paulh@moodle.com>

lang/en/reportbuilder.php

@@ -47,6 +47,7 @@ $string['audienceadded'] = 'Added audience \'{$a}\'';
 $string['audiencedeleted'] = 'Deleted audience \'{$a}\'';
 $string['audiencemultiselectpostfix'] = '{$a->elements} plus {$a->morecount} more';
 $string['audiencenotsaved'] = 'Audience not saved';
+$string['audiencesaved'] = 'Audience saved';
 $string['columnadded'] = 'Added column \'{$a}\'';
 $string['columnaggregated'] = 'Aggregated column \'{$a}\'';
 $string['columndeleted'] = 'Deleted column \'{$a}\'';

reportbuilder/amd/src/audience.js

@@ -32,6 +32,7 @@ import {add as addToast} from 'core/toast';
 import {deleteAudience} from 'core_reportbuilder/local/repository/audiences';
 import * as reportSelectors from 'core_reportbuilder/local/selectors';
 import {loadFragment} from 'core/fragment';
+import {markFormAsDirty} from 'core_form/changechecker';
 
 let reportId = 0;
 let contextId = 0;
@@ -61,7 +62,9 @@ const addAudienceCard = (className, title) => {
             const audienceCard = Templates.appendNodeContents(audiencesContainer, html, js)[0];
             const audienceEmptyMessage = audiencesContainer.querySelector(reportSelectors.regions.audienceEmptyMessage);
 
-            initAudienceCardForm(audienceCard);
+            const audienceForm = initAudienceCardForm(audienceCard);
+            // Mark as dirty new audience form created to prevent users leaving the page without saving it.
+            markFormAsDirty(audienceForm.getFormNode());
             audienceEmptyMessage.classList.add('hidden');
 
             return getString('audienceadded', 'core_reportbuilder', title);
@@ -120,6 +123,9 @@ const initAudienceCardForm = audienceCard => {
         audienceDescription.innerHTML = data.detail.description;
 
         closeAudienceCardForm(audienceCard);
+
+        return getString('audiencesaved', 'core_reportbuilder')
+            .then(addToast);
     });
 
     // If cancelling the form, close the card or remove it if it was never created.

reportbuilder/templates/local/audience/form.mustache

@@ -45,13 +45,21 @@
                 </span>
                 <span class="ml-auto">
                     {{#canedit}}
-                        <button class="btn btn-link px-0" data-action="edit-audience" {{^description}}disabled{{/description}}>
+                        <button class="btn btn-link px-0"
-                            {{#pix}} i/settings, core, {{#str}} editaudience, core_reportbuilder, {{title}} {{/str}} {{/pix}}
+                                data-action="edit-audience"
+                                title="{{#str}} editaudience, core_reportbuilder, {{title}} {{/str}}"
+                                aria-label="{{#str}} editaudience, core_reportbuilder, {{title}} {{/str}}"
+                                {{^description}}disabled{{/description}}>
+                            {{#pix}} i/settings, core {{/pix}}
                         </button>
                     {{/canedit}}
                     {{#candelete}}
-                        <button class="btn btn-link px-0 mr-2" data-action="delete-audience" {{^description}}disabled{{/description}}>
+                        <button class="btn btn-link px-0 mr-2"
-                            {{#pix}} i/trash, core, {{#str}} deleteaudience, core_reportbuilder, {{title}} {{/str}} {{/pix}}
+                                data-action="delete-audience"
+                                title="{{#str}} deleteaudience, core_reportbuilder, {{title}} {{/str}}"
+                                aria-label="{{#str}} deleteaudience, core_reportbuilder, {{title}} {{/str}}"
+                                {{^description}}disabled{{/description}}>
+                            {{#pix}} i/trash, core {{/pix}}
                         </button>
                     {{/candelete}}
                 </span>

reportbuilder/templates/local/settings/area.mustache

@@ -72,7 +72,7 @@
     }
 }}
 
-<div class="reportbuilder-sidebar-settings sidebar collapse notransition" id="report-settings" style="min-width: 350px;">
+<div class="reportbuilder-sidebar-settings sidebar collapse" id="report-settings" style="min-width: 350px;">
     <div class="dropdown d-flex justify-content-end mb-2">
         <div class="mb-3 mt-2 mt-md-0 ml-md-3 w-100">
 

reportbuilder/templates/toggle_card.mustache

@@ -46,7 +46,7 @@
             </button>
         </div>
     </div>
-    <div id="{{$id}}{{/id}}" class="notransition collapse {{$collapsed}}show{{/collapsed}}" aria-labelledby="{{$id}}{{/id}}-heading">
+    <div id="{{$id}}{{/id}}" class="collapse {{$collapsed}}show{{/collapsed}}" aria-labelledby="{{$id}}{{/id}}-heading">
         <div class="card-body p-0">
             {{$body}}{{/body}}
         </div>

reportbuilder/tests/behat/audience.feature

@@ -23,24 +23,25 @@ Feature: Configure access to reports based on intended audience
     And the following "permission overrides" exist:
       | capability                    | permission | role    | contextlevel | reference |
       | moodle/reportbuilder:editall  | Allow      | manager | System       |           |
-    And I log in as "manager1"
+    And I am on the "My report" "reportbuilder > Editor" page logged in as "manager1"
-    And I am on the "My report" "reportbuilder > Editor" page logged in as "admin"
     And I click on the "Access" dynamic tab
     And I should see "Nothing to display"
     And I click on the "Audience" dynamic tab
     And I should see "Add an audience to this report"
     Then I click on "Add audience 'Manually added users'" "link"
+    And I should see "Added audience 'Manually added users'"
     And I set the field "Add users manually" to "User 1,User 3"
     And I press "Save changes"
+    And I should see "Audience saved"
     And I should see "User 1"
     And I should not see "User 2"
     And I should see "User 3"
     And I should not see "Add an audience to this report"
     And I click on the "Access" dynamic tab
-    And I should see "User 1"
+    # It would be better to reference the report table directly, but we can't because of MDL-73011.
-    And I should not see "User 2"
+    And I should see "User 1" in the "[role=tabpanel].active" "css_element"
-    And I should see "User 3"
+    And I should not see "User 2" in the "[role=tabpanel].active" "css_element"
-    And I log out
+    And I should see "User 3" in the "[role=tabpanel].active" "css_element"
 
   Scenario: Configure report audience with has system role audience type
     Given the following "roles" exist:
@@ -49,36 +50,82 @@ Feature: Configure access to reports based on intended audience
     And the following "role assigns" exist:
       | user    | role     | contextlevel | reference |
       | user2   | testrole | System       |          |
-    And I log in as "admin"
     And I am on the "My report" "reportbuilder > Editor" page logged in as "admin"
     And I click on the "Audience" dynamic tab
-    Then I click on "Add audience 'Assigned system role'" "link"
+    When I click on "Add audience 'Assigned system role'" "link"
+    And I should see "Added audience 'Assigned system role'"
     And I set the field "Select a role" to "Test role"
     And I press "Save changes"
+    Then I should see "Audience saved"
     And I should see "Test role"
-    And I log out
+    And I should not see "Add an audience to this report"
+    And I click on the "Access" dynamic tab
+    # It would be better to reference the report table directly, but we can't because of MDL-73011.
+    And I should not see "User 1" in the "[role=tabpanel].active" "css_element"
+    And I should see "User 2" in the "[role=tabpanel].active" "css_element"
+    And I should not see "User 3" in the "[role=tabpanel].active" "css_element"
 
   Scenario: Configure report audience with Member of cohort audience type
-    And the following "cohorts" exist:
+    Given the following "cohorts" exist:
       | name    | idnumber |
       | Cohort1 | cohort1  |
-    And I log in as "admin"
+    And the following "cohort members" exist:
+      | cohort  | user  |
+      | cohort1 | user3 |
     And I am on the "My report" "reportbuilder > Editor" page logged in as "admin"
     And I click on the "Audience" dynamic tab
-    Then I click on "Add audience 'Member of cohort'" "link"
+    When I click on "Add audience 'Member of cohort'" "link"
+    And I should see "Added audience 'Member of cohort'"
     And I set the field "Select members from cohort" to "Cohort1"
     And I press "Save changes"
+    Then I should see "Audience saved"
     And I should see "Cohort1"
-    And I log out
+    And I should not see "Add an audience to this report"
+    And I click on the "Access" dynamic tab
+    # It would be better to reference the report table directly, but we can't because of MDL-73011.
+    And I should not see "User 1" in the "[role=tabpanel].active" "css_element"
+    And I should not see "User 2" in the "[role=tabpanel].active" "css_element"
+    And I should see "User 3" in the "[role=tabpanel].active" "css_element"
 
   Scenario: Configure report audience with Member of cohort audience type with no cohorts available
-    Given I log in as "admin"
+    Given I am on the "My report" "reportbuilder > Editor" page logged in as "admin"
-    And I am on the "My report" "reportbuilder > Editor" page logged in as "admin"
     And I click on the "Audience" dynamic tab
     Then "Add audience 'All users'" "link" should exist
     # This audience type should be disabled because there are no cohorts available.
     And "Add audience 'Member of cohort'" "link" should not exist
 
+  Scenario: Delete report audience
+    Given I am on the "My report" "reportbuilder > Editor" page logged in as "admin"
+    And I click on the "Audience" dynamic tab
+    And I click on "Add audience 'All users'" "link"
+    And I press "Save changes"
+    When I click on "Delete audience 'All users'" "button"
+    And I click on "Delete" "button" in the "Delete audience 'All users'" "dialogue"
+    Then I should see "Deleted audience 'All users'"
+    And I should see "Add an audience to this report"
+
+  Scenario: Edit report audience with manually added users audience type
+    Given I am on the "My report" "reportbuilder > Editor" page logged in as "admin"
+    And I click on the "Access" dynamic tab
+    And I should see "Nothing to display"
+    And I click on the "Audience" dynamic tab
+    And I should see "Add an audience to this report"
+    Then I click on "Add audience 'Manually added users'" "link"
+    And I set the field "Add users manually" to "User 1,User 3"
+    And I press "Save changes"
+    And I press "Edit audience 'Manually added users'"
+    And I set the field "Add users manually" to "User 2"
+    And I press "Save changes"
+    And I should see "Audience saved"
+    # It would be better to reference the report table directly, but we can't because of MDL-73011.
+    And I should not see "User 1" in the "[role=tabpanel].active" "css_element"
+    And I should see "User 2" in the "[role=tabpanel].active" "css_element"
+    And I should not see "User 3" in the "[role=tabpanel].active" "css_element"
+    And I click on the "Access" dynamic tab
+    And I should not see "User 1" in the "[role=tabpanel].active" "css_element"
+    And I should see "User 2" in the "[role=tabpanel].active" "css_element"
+    And I should not see "User 3" in the "[role=tabpanel].active" "css_element"
+
   Scenario: View report as a user with no edit capability and set in the report audience
     Given the following "core_reportbuilder > Reports" exist:
       | name              | source                                   |
@@ -114,7 +161,6 @@ Feature: Configure access to reports based on intended audience
     And I navigate to "Reports > Report builder > Custom reports" in site administration
     And I should not see "My second report"
     And I click on "My report" "link" in the "My report" "table_row"
-    And I log out
 
   Scenario: View report as a user with edit capability
     Given the following "core_reportbuilder > Reports" exist:
@@ -160,7 +206,6 @@ Feature: Configure access to reports based on intended audience
     And I should not see "My second report"
     And I should see "My user1 report"
     And I click on "My report" "link" in the "My report" "table_row"
-    And I log out
 
   Scenario: View report as a user with editall capability
     Given the following "core_reportbuilder > Reports" exist:
@@ -184,4 +229,3 @@ Feature: Configure access to reports based on intended audience
     And I should see "My report"
     Then I click on "My second report" "link" in the "My second report" "table_row"
     And I should see "Email address"
-    And I log out

theme/boost/scss/moodle/reportbuilder.scss

@@ -117,14 +117,6 @@
     @include reportbuilder-scrollbar;
 }
 
-/* Remove transition effects. */
-.reportbuilder-wrapper .notransition {
-    -webkit-transition: none !important; /* stylelint-disable-line declaration-no-important */
-    -moz-transition: none !important; /* stylelint-disable-line declaration-no-important */
-    -o-transition: none !important; /* stylelint-disable-line declaration-no-important */
-    transition: none !important; /* stylelint-disable-line declaration-no-important */
-}
-
 /* Customreport settings */
 .reportbuilder-sidebar-settings {
     @include media-breakpoint-up(md) {

theme/boost/style/moodle.css

@@ -20678,17 +20678,6 @@ div.editor_atto_toolbar button .icon {
     background-color: white;
     border-left: 5px solid #fff; }
 
-/* Remove transition effects. */
-.reportbuilder-wrapper .notransition {
-  -webkit-transition: none !important;
-  /* stylelint-disable-line declaration-no-important */
-  -moz-transition: none !important;
-  /* stylelint-disable-line declaration-no-important */
-  -o-transition: none !important;
-  /* stylelint-disable-line declaration-no-important */
-  transition: none !important;
-  /* stylelint-disable-line declaration-no-important */ }
-
 /* Customreport settings */
 @media (min-width: 768px) {
   .reportbuilder-sidebar-settings {

theme/classic/style/moodle.css

@@ -20624,17 +20624,6 @@ div.editor_atto_toolbar button .icon {
     background-color: white;
     border-left: 5px solid #fff; }
 
-/* Remove transition effects. */
-.reportbuilder-wrapper .notransition {
-  -webkit-transition: none !important;
-  /* stylelint-disable-line declaration-no-important */
-  -moz-transition: none !important;
-  /* stylelint-disable-line declaration-no-important */
-  -o-transition: none !important;
-  /* stylelint-disable-line declaration-no-important */
-  transition: none !important;
-  /* stylelint-disable-line declaration-no-important */ }
-
 /* Customreport settings */
 @media (min-width: 768px) {
   .reportbuilder-sidebar-settings {