Quiz creation side panels improvements #12819

nucleogenesis · 2024-11-11T20:46:58Z

Summary

Create-new-quiz---Kolibri.1.mp4

When working on quiz creation I created a SectionSidePanel component to kind of wrap around all of the various side-panel kinds but realized a bit too late on that it wasn't really necessary and added some unnecessary complexity.

This instead begins organizing the code so that each of the child routes' components for the side panels are wrapped in the SidePanelModal component which results in them being rendered into the <router-view/> in the quiz creation root route.

Some motivations for this were:

Removing unnecessary indirection around when/how/where components define what to do re: back and close behavior
Providing a better example for handling side panels w/ routing for Lessons
Much easier to put things into the Side panel title section

Reviewer guidance

QA

No regressions in the quiz creation side panels - section ordering, section editing, resource selection (search,bookmarks,etc), question replacement.
Ensure that "are you sure" modals are triggered and that they're "Yes/No" handling works properly.
Notably - "Section Editor <Click 'Add Questions'> -> Resource Selection" - make a change to your selection, then hit the browser back button until you get the "Are you sure" modal -- note that when you confirm it, you are returned correctly to the "Section Editor" side panel. However, had you clicked the "X" to close it, then you'd just go straight to the root page.

Code review

Whaddya think overall?

Testing checklist

Contributor has fully tested the PR manually
If there are any front-end changes, before/after screenshots are included
Critical user journeys are covered by Gherkin stories
Critical and brittle code paths are covered by unit tests

PR process

PR has the correct target branch and milestone
PR has 'needs review' or 'work-in-progress' label
If PR is ready for review, a reviewer has been added. (Don't use 'Assignees')
If this is an important user-facing change, PR or related issue has a 'changelog' label
If this includes an internal dependency change, a link to the diff is provided

Reviewer checklist

PR is fully functional
PR has been tested for accessibility regressions
External dependency files were updated if necessary (yarn and pip)
Documentation is updated
Contributor is in AUTHORS.md

github-actions · 2024-11-11T21:18:05Z

Build Artifacts

Asset type	Download link
PEX file	kolibri-0.18.0.dev0_git.20241120235751.pex
Windows Installer (EXE)	kolibri-0.18.0.dev0+git.20241120235751-windows-setup-unsigned.exe
Debian Package	kolibri_0.18.0.dev0+git.20241120235751-0ubuntu1_all.deb
Mac Installer (DMG)	kolibri-0.18.0.dev0+git.20241120235751.dmg
Android Package (APK)	kolibri-0.18.0.dev0+git.20241120235751-0.1.4-debug.apk
TAR file	kolibri-0.18.0.dev0+git.20241120235751.tar.gz
WHL file	kolibri-0.18.0.dev0+git.20241120235751-py2.py3-none-any.whl

pcenov · 2024-11-13T12:16:42Z

Hi @nucleogenesis,

Here's a list with the regressions I was able to spot:

Add questions - The "Are you sure you want to leave" modal is correctly displayed when I click the Close icon after having selected questions, but clicking the Continue button doesn't immediately close the side panel while it should:

continue.mp4

Add questions - Missing 'Back' arrow after having navigated through a channel's folders. I guess one can still use the browser's back button or the breadcrumb navigation, but it's inconsistent with the implementation at 'Edit section' and not very convenient:

missing.back.arrow.mp4

Add questions - There's not enough spacing between the 'Add questions to 'Section 1' text and the 'Current number of questions in this section'. Long titles will also not be displayed nicely:

Edit section - The "Are you sure you want to leave" modal is displayed when I click the Back button after having selected some questions. In that case it should be possible to go back to the 'Section settings' page:

areusure.mp4

Edit section > Add more questions - The 'Close' icon is missing so I can't close the side panel right away if I want to:

close.mp4

Edit section - Changes made to the section title and description are being saved without having clicked the Apply settings button:

changes.should.not.be.saved.mp4

All the modals are not displayed correctly:

replace.modal.mp4

Selecting a section from the '...' button is not working:

navigation.issues.mp4

AlexVelezLl

Just a small consideration regarding the naming of these components, and the "go back" behaviour that is not completely clear to me.

AlexVelezLl · 2024-11-12T16:01:24Z

kolibri/plugins/coach/assets/src/views/plan/CreateExamPage/ResourceSelection.vue

@@ -656,7 +664,7 @@
      }

      function handleConfirmClose() {
-        context.emit('closePanel');
+        this.$router.back();


I wonder what is the difference between this this.$router.back() and the use of the this.prevRoute? We could add a comment there to explain the purpose of each and why we have different patterns for confirmClose and just closing the side panel

The whole thing was kind of confusing so I've included some updates to SidePanelModal that made this a lot easier to deal with. Lemme know what you think when you can.

kolibri/plugins/coach/assets/src/views/plan/CreateExamPage/ReplaceQuestions.vue

nucleogenesis · 2024-11-14T20:58:20Z

@AlexVelezLl @pcenov I have updated the PR and addressed the feedback.

Edit section - Changes made to the section title and description are being saved without having clicked the Apply settings button:

This is intentional - when you click "Add questions" we decided to skip confirming "Are you sure?" and just auto-save the user's changes when they go to add questions

KModals are all fixed

Here is an example of the navigation - I have made changes such that the Back arrow shows AND the Close icon shows in the context of going "Section editor -> Add questions" - so the user can always close out from there but if they use the back arrow, they'll eventually go back to the "Section editor" from the "Add questions" (with the confirmation modal if they've made changes, but no confirmation modal if they have not).

fix-navigation.mp4

I made a change that seems to avoid the "redundant navigation" error. However, I have been able to successfully select any section:

fix-section-selection-in-overflow.mp4

pcenov · 2024-11-18T11:28:13Z

Hi @nucleogenesis - it seems that there are some conflicts that must be resolved and there are no new build assets for me to test at this point, so could you take a look?

AlexVelezLl

Looks good to me! Code changes makes sense to me, and I havent spotted anything weird, I will left the final approve to QA's review. Thanks @nucleogenesis!

…al props

pcenov · 2024-11-21T13:53:13Z

Hi @nucleogenesis, looks much better already!

The following issues are still not fixed or partially fixed:

Add questions - There's not enough spacing between the 'Add questions to 'Section 1' text and the 'Current number of questions in this section'. Long titles will also not be displayed nicely:

2024-11-21_14-29-21.mp4

All the modals are now displayed correctly but always with a tiny scroll-bar:

Selecting a section from the '...' button is still not working properly as I can't see the first 3 sections when I shrink the window:

2024-11-21_13-38-42.mp4

Edit section - Changes made to the section title and description are being saved without having clicked the Apply settings button:

This is intentional - when you click "Add questions" we decided to skip confirming "Are you sure?" and just auto-save the user's changes when they go to add questions

I believe that it would be fine to skip the confirmation only if the user has actually added new questions. But if the user has clicked on "Add questions" and then went back to "Section settings" without having added any new questions or clicked the "Apply settings" button, then it's not ok to auto-save as it's not consistent with the scenario where one goes to "Section settings" makes some changes, hits the close icon and sees the "Are you sure" modal:

2024-11-21_15-31-48.mp4

Notably - "Section Editor <Click 'Add Questions'> -> Resource Selection" - make a change to your selection, then hit the browser back button until you get the "Are you sure" modal -- note that when you confirm it, you are returned correctly to the "Section Editor" side panel. However, had you clicked the "X" to close it, then you'd just go straight to the root page.

When following your guidance for this scenario I am always going back to the root page, and not to the "Section settings" side panel:

2024-11-21_15-51-03.mp4

AlexVelezLl

Thanks Jacob! I have spotted a couple of weird behaviours with the back button, but they are mostly questions to confirm if this is the expected behaviour

AlexVelezLl · 2024-11-21T14:39:29Z

kolibri/plugins/coach/assets/src/views/quizzes/CreateExamPage/SidePanels/ResourceSelection.vue

@@ -491,9 +485,6 @@
        loadingMore,
      } = useQuizResources({ topicId, practiceQuiz: selectPracticeQuiz.value });

-      const { searchTerms, search } = useBaseSearch({ descendant: topic });


This wasnt being used before?

AlexVelezLl · 2024-11-21T14:53:34Z

kolibri/plugins/coach/assets/src/views/quizzes/CreateExamPage/SidePanels/ResourceSelection.vue

-    },
-    */
+        selectAllIsVisible() {
+          TO BE IMPLEMENTED IN https://github.com/learningequality/kolibri/issues/11734


This issue is already closed. Its still valid? Or do we need to remove this?

AlexVelezLl · 2024-11-21T15:17:21Z

kolibri/plugins/coach/assets/src/views/quizzes/CreateExamPage/SidePanels/ResourceSelection.vue

+            // arrow in that case. Here we check if there is a valid prevRoute and if there is,
+            // then we know we can go back if we have a topicId or are showing bookmarks
+            // TODO When we add search it'll probably want to be here too
+            (prevRoute.value.name && (topicId.value || showBookmarks.value)),


I think its a very edge case, but with this condition, if I reload the page, prevRoute.value.name is always empty, and I completely lose the back button and doesnt appear again even if I navigate through the tree:

Compartir.pantalla.-.2024-11-21.10_12_54.mp4

Thank you for this - it shouldn't show up on page load, but it should once you start navigating for sure

AlexVelezLl · 2024-11-21T15:24:27Z

kolibri/plugins/coach/assets/src/views/quizzes/CreateExamPage/SidePanels/ResourceSelection.vue

@@ -848,6 +871,12 @@
      focusFirstEl() {
        this.$refs.textbox.focus();
      },
+      handleGoBack() {
+        this.$router.back();


I was wondering what does the "go back" button means when we are navigating through the tree. If it means the same behaviour as the browser back button, or if it means go back in the tree. From my perspective I would think that if I click the back button while searching through the tree, it always take me back to the parent folder, but this doesnt happen if we click on the breadcrumbs:

Compartir.pantalla.-.2024-11-21.10_23_16.mp4

It also happens when we open the section editor:

Compartir.pantalla.-.2024-11-21.10_26_55.mp4

github-actions bot added APP: Coach Re: Coach App (lessons, quizzes, groups, reports, etc.) DEV: frontend labels Nov 11, 2024

nucleogenesis mentioned this pull request Nov 11, 2024

Update SearchFiltersPanel for Coach #12759

Merged

2 tasks

nucleogenesis requested review from rtibbles, AlexVelezLl and pcenov November 11, 2024 22:30

nucleogenesis marked this pull request as ready for review November 11, 2024 22:31

AlexVelezLl reviewed Nov 13, 2024

View reviewed changes

nucleogenesis linked an issue Nov 13, 2024 that may be closed by this pull request

Coach Quiz Side Panels need refactoring #12840

Open

3 tasks

nucleogenesis mentioned this pull request Nov 13, 2024

Remove labels fetching for Channels in useBaseSearch #12841

Open

3 tasks

github-actions bot added the APP: Learn Re: Learn App (content, quizzes, lessons, etc.) label Nov 14, 2024

AlexVelezLl reviewed Nov 18, 2024

View reviewed changes

This was referenced Nov 18, 2024

Lesson Summary children routes refactor #12852

Merged

Initial implementation of the Resource management side panel #12857

Merged

rtibbles assigned marcellamaki Nov 19, 2024

nucleogenesis added 3 commits November 20, 2024 14:53

move quiz creation side panels to own subdir, fix refs

0ec459e

updates to SidePanelModal to differentiate goBack & closePanel actions

72894ca

remove sectionsidepanel; update coach quizzes to use new sidepanelmod…

033e53c

…al props

nucleogenesis force-pushed the quiz-routing-side-panels branch from 77af83c to 033e53c Compare November 20, 2024 23:57

AlexVelezLl reviewed Nov 21, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Quiz creation side panels improvements #12819

Quiz creation side panels improvements #12819

nucleogenesis commented Nov 11, 2024 •

edited

Loading

github-actions bot commented Nov 11, 2024 •

edited

Loading

pcenov commented Nov 13, 2024

AlexVelezLl left a comment

AlexVelezLl Nov 12, 2024

nucleogenesis Nov 14, 2024

nucleogenesis commented Nov 14, 2024

pcenov commented Nov 18, 2024

AlexVelezLl left a comment

pcenov commented Nov 21, 2024

AlexVelezLl left a comment

AlexVelezLl Nov 21, 2024

AlexVelezLl Nov 21, 2024

AlexVelezLl Nov 21, 2024

nucleogenesis Nov 21, 2024

AlexVelezLl Nov 21, 2024 •

edited

Loading

AlexVelezLl Nov 21, 2024

Quiz creation side panels improvements #12819

Are you sure you want to change the base?

Quiz creation side panels improvements #12819

Conversation

nucleogenesis commented Nov 11, 2024 • edited Loading

Summary

Reviewer guidance

QA

Code review

Testing checklist

PR process

Reviewer checklist

github-actions bot commented Nov 11, 2024 • edited Loading

pcenov commented Nov 13, 2024

AlexVelezLl left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nucleogenesis commented Nov 14, 2024

pcenov commented Nov 18, 2024

AlexVelezLl left a comment

Choose a reason for hiding this comment

pcenov commented Nov 21, 2024

AlexVelezLl left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlexVelezLl Nov 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nucleogenesis commented Nov 11, 2024 •

edited

Loading

github-actions bot commented Nov 11, 2024 •

edited

Loading

AlexVelezLl Nov 21, 2024 •

edited

Loading