Integrate the "useSearch" composable and search filters panel into the results list in quizzes #13083

AllanOXDi · 2025-02-14T17:23:33Z

Summary

Integrate search filters panel to quiz selection
Add a new immersive header to the side panel modal component.

References

Closes #13035

Reviewer guidance

Go to quiz > New quiz >select quiz.
Click on the filter button from the first page on the side panel, or from within any topic
Play and try any possible combination with different filters, browsing search results topics.

github-actions · 2025-02-14T17:55:45Z

Build Artifacts

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

akolson · 2025-02-21T08:49:06Z

kolibri/plugins/coach/assets/src/routes/examRoutes.js

@@ -16,12 +16,14 @@ import SelectionIndex from '../views/common/resourceSelection/subPages/Selection
 import SelectFromChannels from '../views/common/resourceSelection/subPages/SelectFromTopicTree.vue';


Let's change the import name to SelectFromTopicTree so there is consistency.

…rch results

marcellamaki · 2025-03-04T15:58:42Z

Hi @AllanOXDi - I've only done a very preliminary read through and quick manual QA, and while I think there will a few adjustments that need to be made, my initial takeaway is that this is looking great! Going through the basic workflow I can search, see my results, open search result folders and go back to search, and add questions. Well done. I'll come back later today with more focused feedback but overall, a very strong start :) nice work.

marcellamaki · 2025-03-04T23:29:45Z

kolibri/plugins/coach/assets/src/views/common/resourceSelection/UpdatedResourceSelection.vue

@@ -21,7 +21,7 @@
        :contentCheckboxDisabled="contentCheckboxDisabled"
        :contentCardLink="contentLink"
        :contentCardMessage="contentCardMessage"
-        :showRadioButtons="!multi"
+        :showRadioButtons="showRadioButton"


I'm a little bit confused about what's happening here with the radio buttons. My thinking is that maybe there is some pre-existing 0.17 work that allows for selection for practice quizzes, that is colliding a bit with this new scenario, of making individual questions selectable. I realized in the bug bash last week that I hadn't fully specced out the updates to practice quizzes, and I think that can be done in follow up since it's about selection of resources in general, not just in search results. But here in your work, we do want to have the selection be a checkbox (if selecting the whole resource) rather than a radio button

If there's anything you want to chat through about this, let me know and we can outline together what the scenarios need to be :)

It's kinder confusing because I see on the spec it's a radio button. Since the goal here is to allow selecting individual questions, switching to checkboxes for whole-resource selection sounds like the right approach.

I think understanding the context of the multi prop would be a great starting point. From a quick read through, the showRadioButton is some kind of a toggle between the use of radio or checkboxes. I guess its mean to offer flexibility for the card.

The multi props context in the SelectFromBookmarks.vue and SelectFromTopics.vue is !settings?.selectPracticeQuiz(which i think means practice quiz is selectable or not?) so using target === SelectionTarget.QUIZ could break functionality, I think?

But back to Marcella's point, its probably best to chat about this "in-person" as it could involve a few moving pieces. Hopefully that helps

marcellamaki · 2025-03-04T23:31:03Z

kolibri/plugins/coach/assets/src/views/common/resourceSelection/UpdatedResourceSelection.vue

@@ -235,6 +245,9 @@
        }
        return ViewMoreButtonStates.NO_MORE;
      },
+      showRadioButton() {
+        return this.target === SelectionTarget.QUIZ;


Using SelectionTarget here is a great idea - but I think we might need to extend it (see the comment about re: practice quizzes, which might be easier to just talk through " in person")

AlexVelezLl

Hi @AllanOXDi! I was just passing by, and just noted a couple of things to be aware of :).

AlexVelezLl · 2025-03-05T16:34:05Z

kolibri/plugins/coach/assets/src/views/common/resourceSelection/subPages/SelectionIndex.vue

@@ -23,6 +23,7 @@
        v-if="target === SelectionTarget.QUIZ && !settings.selectPracticeQuiz"
        class="mb-24"
        :settings="settings"
+        :hideSetting="true"


Is there any reason why do we want to hide the settings button here? In the specs we have both the settings and the search button in the selection index page.

I have a looked at the spec I did not see where we are showing it
https://www.figma.com/design/lVJt5ukOrxS9qay0rXy7GC/0.18-Coach-updates?node-id=107-28854&t=74AwO4l2G3YAYHd8-0

Ah thats just for practice quizzes then. Not for all instances of Selection Index. With this we are currently breaking this spec https://www.figma.com/design/lVJt5ukOrxS9qay0rXy7GC/0.18-Coach-updates?node-id=2189-178270&t=CUt0c5tq2iT8oGZD-0

AlexVelezLl · 2025-03-05T18:43:41Z

packages/kolibri-common/composables/useBaseSearch.js

+
+      const params = get(route).params;
+      if (params?.quizId) {
+        getParams.contains_quiz = true;


This is not always true, we should set this filter to true just for practice quizzes, for example this is a non-practice quiz that appears while browsing the folder tree:

If I search by its name, It wont appear:

This doesnt happen in lessons (i.e. its not a problem with the keywords filter):

(IMO we should pass these filters in other way, we shouldnt rely on route.params.quizId for that, since this is a generic composable that can be used in multiple contexts, and the useBaseSearch composable shouldnt know the logic of when to apply the contains_quiz to true)

AlexVelezLl · 2025-03-05T18:45:19Z

packages/kolibri-common/composables/useBaseSearch.js

@@ -253,7 +254,7 @@ export default function useBaseSearch({
    if (get(displayingSearchResults)) {
      getParams.max_results = 25;
      const terms = get(searchTerms);
-      set(searchResultsLoading, true);


Why should we remove this loading state? This will make that we dont see the circular loader when the search is loading.

AlexVelezLl · 2025-03-05T18:56:32Z

...zes/CreateExamPage/sidePanels/QuizResourceSelection/subPages/SelectFromQuizSearchResults.vue

+        type: Function,
+        required: true,
+      },
+      target: {


Non-blocking but to be aware of. Should these two new subpages be rather a common subpage that we can use for both lessons and quizzes? I have tried to keep the common subpages in common/resourceSelection/subPages to avoid too much code duplication, although if the quizzes search results and search filters pages differs too much from the lessons search results, then it would make sense to have these two components separated. But if this is the case, then we dont need these target prop, since this component will be always rendered in the quizzes context.

…depanel

AlexVelezLl · 2025-03-06T19:11:24Z

packages/kolibri-common/composables/useBaseSearch.js

@@ -290,8 +291,8 @@ export default function useBaseSearch({
        set(_results, data.results || []);
        set(more, data.more);
        _setAvailableLabels(data.labels);


This set(searchResultsLoading, false); should be inside the then callback so we set the search results to false just after we have successfully fetched the search results.

I am leaving this out because once its left within api call the loading state does not stop and no data displays.

No @AllanOXDi, we cant left this out, because we are breaking the loading state of the search results fetch. We use this composable in Learn too. And we can see here that the loading state is gone now :').

Before this PR:

Compartir.pantalla.-.2025-03-07.14_23_12.mp4

After, with this specific change:

Compartir.pantalla.-.2025-03-07.14_24_44.mp4

When we make changes on something that was already implemented, we also need to bear in mind how is this affecting to all the parts of the code that uses this composable/method/component, and why the current implementation is like that, to know if we can safely change it :), unfortunately this is not the case :/

Let me know what is the error you are seeing with this, and we can figure out how can we solve it without breaking the loading state. We have pretty much the same behaviour in Lessons so we can take inspiration from the implementation we have in lessons and figure out why in Lessons work fine, and here not.

I just found the error you are seeing. To avoid the infinite loading, you will need to copy this subpageLoading computed property we have in lessons, and skip the loading if the current page is the search filters page fc945b4#diff-19b8c2b6b4d2b12a66a9d9a35c3a4e6b6312c432540bfbab30346aaa9ae83ea0R140. That is how it is solved in lessons and we can copy this to quizzes too :)

AlexVelezLl

Thanks @AllanOXDi! Is looking good! Just left some observations on things that can be still polished a little bit more.

AlexVelezLl · 2025-03-07T19:49:30Z

...bri/plugins/coach/assets/src/views/common/resourceSelection/subPages/SelectFromTopicTree.vue

@@ -59,9 +59,10 @@
      :hasMore="hasMore"
      :fetchMore="fetchMore"
      :loadingMore="loadingMore"
-      :multi="!settings?.selectPracticeQuiz"
+      :multi="settings?.selectPracticeQuiz"


The correct condition here is !settings?.selectPracticeQuiz, this condition negated.

When we go through the "select practice quiz" flow, the intention is that we select just one resource, thats why this multi exists, so just when we are selecting practice quiz, the multi condition should be false. Thats why this condition should be :multi="!settings?.selectPracticeQuiz"

With this, we are being able to select multiple resources when selecting a practice quiz:

Note the difference when we go to create a new quiz as "select quiz" vs "create new quiz"

AlexVelezLl · 2025-03-07T19:50:09Z

...bri/plugins/coach/assets/src/views/common/resourceSelection/subPages/SelectFromTopicTree.vue

      :selectionRules="selectionRules"
      :selectAllRules="selectAllRules"
+      :target="target"


We dont need this target prop anymore.

AlexVelezLl · 2025-03-07T19:53:44Z

...ins/coach/assets/src/views/quizzes/CreateExamPage/sidePanels/QuizResourceSelection/index.vue

@@ -385,6 +405,11 @@
          },
          annotator: annotateTopicsWithDescendantCounts,
        },
+        quizSearch: {


We dont have this quizSearch in the useResourceSeleciton, what is it for?

AlexVelezLl · 2025-03-07T20:10:00Z

packages/kolibri-common/composables/useBaseSearch.js

@@ -290,8 +291,8 @@ export default function useBaseSearch({
        set(_results, data.results || []);
        set(more, data.more);
        _setAvailableLabels(data.labels);


I just found the error you are seeing. To avoid the infinite loading, you will need to copy this subpageLoading computed property we have in lessons, and skip the loading if the current page is the search filters page fc945b4#diff-19b8c2b6b4d2b12a66a9d9a35c3a4e6b6312c432540bfbab30346aaa9ae83ea0R140. That is how it is solved in lessons and we can copy this to quizzes too :)

AlexVelezLl · 2025-03-07T20:15:10Z

...views/quizzes/CreateExamPage/sidePanels/QuizResourceSelection/subPages/SearchQuizFilters.vue

@@ -0,0 +1,138 @@
+<template>
+


Could we have a follow up issue for unifying these two components with the ones we have in lessons? I think that there are few differences to what we have in lessons and there is some non-trivial logic that would be good to have de-duplicated cc: @marcellamaki

Yes! there will be some planned, incremental refactoring that we do that I'm planning for the 0.18 patch releases to further clean up and consolidate some logic, without getting into the weeds on everything right now. It's a good suggestion to do that for this code, @AlexVelezLl

AlexVelezLl · 2025-03-07T20:23:01Z

packages/kolibri-common/composables/useBaseSearch.js

+
+      const query = get(route).query;
+      if (query?.filter_quiz) {
+        getParams.contains_quiz = true;


This is still not working well, I am now able to see non-exercise resources in search.

Again, I dont think the route query should be the way to do this. We can cohack to find a better solution!

marcellamaki

Hi @AllanOXDi - it seems like Alex has shared some helpful notes here. I think the practice quizzes are still a bit of a sticking point. It sounds like the two of you will connect on monday, but please let me know if it's helpful for me to jump into a call to clarify anything.

For the scope of this issue:

let's focus only on the "create" quiz workflow (non-practice quizzes) which should be a checkbox/multi selection option integrated with the topic/resource/question selection workflow.
I'm not concerned about the changes here causing "regressions" in practice quizzes, or that workflow not being completed; let's prioritize getting the create quiz workflow ready to go here, and then revisit the practice quizzes separately

Other than that, this seems very close to ready, with just a few suggestions on cleanup. I'll be happy to do another review pass after you two have a cohack, but I anticipate that will address most of the feedback. Thanks again for the work here, @AllanOXDi - it's looking good!

marcellamaki · 2025-03-11T22:29:55Z

@pcenov @radinamatic - this is ready for QA team review, and I'd like to request it goes to the top of the list for Wednesday please. There are some known follow up issues to be filed with Practice Quizzes (aka the "select quiz" workflow), so let's focus QA here just on using search in the regular "new quiz" (or editing a quiz) experience.

I'd also like us to put as much as we can into follow up issues just so we can potentially assign to multiple team members to help keep things moving, and I'll review any feedback comments tomorrow to help sort that out.

pcenov · 2025-03-12T12:28:22Z

Observed the following issues:

The filters should be displayed as active only if there are exercises to which they apply. Currently I can select an active filter only to see "0 results":

filters.should.be.for.exercises.only.mp4

Similarly when I search by keyword I am seeing folders with no available exercises and resources that are not exercises:

show.only.folders.withc.exercises.mp4

After searching by keyword or by applying a filter the title of the page changes to "Select a practice quiz":

select.a.practice.quiz.mp4

If I have already selected all questions within a folder and I search for the same folder it's not displayed as not selectable:

folder.with.already.selected.questions.mp4

marcellamaki · 2025-03-12T14:26:27Z

Thank you @pcenov - I think we may move those tasks to follow up, so that we can distribute them among a few people, but still plan to get them resolved promptly so we can tag a beta release (cc @radinamatic). I am going to do one more code read through, and will follow up @AllanOXDi. Thanks everyone for the effort here with Allan's effort, the close code review, and the QA. nearly there!

marcellamaki

Alright - let's go ahead and get this merged in - thank you @AllanOXDi! I'll sort out follow up issues for the handful of things that Peter mentioned

Feedback addressed, and remaining items will be addressed in follow up issues :)

github-actions bot added APP: Coach Re: Coach App (lessons, quizzes, groups, reports, etc.) DEV: frontend labels Feb 14, 2025

AllanOXDi changed the title ~~adds search filter panel at quiz side panel~~ Integrate the "useSearch" composable and search filters panel into the results list in quizzes Feb 17, 2025

AllanOXDi force-pushed the Integrate-useSearch branch from 5286738 to bbffa9c Compare February 19, 2025 20:23

akolson reviewed Feb 21, 2025

View reviewed changes

marcellamaki self-assigned this Feb 25, 2025

AllanOXDi added 16 commits March 4, 2025 12:23

adds search filter panel at quiz side panel

63de979

transfers search components to the coach common

69f9948

can open search results page on the quiz side panel

24d3740

hides setting button when on quiz sidepanel

81726fd

skips the loading of the sidepanel

f9e9dd2

conditionalise navigation on the sidepanel to cater for quizes

bb2b22d

utilises existing page name constant

fead039

renders search panel using coach common component

2ec1507

renames quiz search components

f9f5cb2

uses existing quiz pagename constant

365454b

changed importation name

c69b5f2

change resource to quizresoucre

d78f02c

fix quiz filter

ab892ca

adds params to api request to filter out practice quizzes

5785ce3

transfers components to quiz subpages

4ecaef4

changes sidepanel header color on navigating through a topic from sea…

54263ec

…rch results

AllanOXDi force-pushed the Integrate-useSearch branch from 60816cb to 54263ec Compare March 4, 2025 10:57

AllanOXDi marked this pull request as ready for review March 4, 2025 10:59

fix test

38646f5

marcellamaki reviewed Mar 4, 2025

View reviewed changes

AlexVelezLl reviewed Mar 5, 2025

View reviewed changes

AllanOXDi added 2 commits March 6, 2025 19:32

displays settings button when navigating through the practice quiz si…

95355e8

…depanel

loading state in usebasesearch

da8b02a

AlexVelezLl reviewed Mar 6, 2025

View reviewed changes

adds a filtering for quiz

8df26a2

AllanOXDi requested review from AlexVelezLl, marcellamaki and akolson and removed request for AlexVelezLl March 7, 2025 16:53

enforces checkboxes instead of radio buttons'

c0ca8ec

AlexVelezLl previously requested changes Mar 7, 2025

View reviewed changes

marcellamaki reviewed Mar 7, 2025

View reviewed changes

AllanOXDi added 3 commits March 11, 2025 14:24

.

47fa41e

fixes inifinite loading in the search

5be03e2

adds filtering the practice quiz

a054d56

AllanOXDi requested review from AlexVelezLl and marcellamaki March 11, 2025 17:50

marcellamaki approved these changes Mar 12, 2025

View reviewed changes

AllanOXDi merged commit a1ce731 into learningequality:develop Mar 12, 2025
36 checks passed

AllanOXDi deleted the Integrate-useSearch branch March 12, 2025 19:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Integrate the "useSearch" composable and search filters panel into the results list in quizzes #13083

Integrate the "useSearch" composable and search filters panel into the results list in quizzes #13083

AllanOXDi commented Feb 14, 2025 •

edited

Loading

github-actions bot commented Feb 14, 2025 •

edited

Loading

akolson Feb 21, 2025

marcellamaki commented Mar 4, 2025

marcellamaki Mar 4, 2025

marcellamaki Mar 4, 2025

AllanOXDi Mar 5, 2025

akolson Mar 5, 2025

marcellamaki Mar 4, 2025

AlexVelezLl left a comment

AlexVelezLl Mar 5, 2025

AllanOXDi Mar 6, 2025

AlexVelezLl Mar 6, 2025

AlexVelezLl Mar 5, 2025

AlexVelezLl Mar 5, 2025

AlexVelezLl Mar 5, 2025

AlexVelezLl Mar 5, 2025

AlexVelezLl Mar 6, 2025

AllanOXDi Mar 7, 2025

AlexVelezLl Mar 7, 2025

AlexVelezLl Mar 7, 2025

AlexVelezLl left a comment

AlexVelezLl Mar 7, 2025

AlexVelezLl Mar 7, 2025

AlexVelezLl Mar 7, 2025

AlexVelezLl Mar 7, 2025

AlexVelezLl Mar 7, 2025

marcellamaki Mar 7, 2025

AlexVelezLl Mar 7, 2025

marcellamaki left a comment

marcellamaki commented Mar 11, 2025

pcenov commented Mar 12, 2025

marcellamaki commented Mar 12, 2025

marcellamaki left a comment

		@@ -16,12 +16,14 @@ import SelectionIndex from '../views/common/resourceSelection/subPages/Selection
		import SelectFromChannels from '../views/common/resourceSelection/subPages/SelectFromTopicTree.vue';

Integrate the "useSearch" composable and search filters panel into the results list in quizzes #13083

Integrate the "useSearch" composable and search filters panel into the results list in quizzes #13083

Conversation

AllanOXDi commented Feb 14, 2025 • edited Loading

Summary

References

Reviewer guidance

github-actions bot commented Feb 14, 2025 • edited Loading

Choose a reason for hiding this comment

marcellamaki commented Mar 4, 2025

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 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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marcellamaki left a comment

Choose a reason for hiding this comment

marcellamaki commented Mar 11, 2025

pcenov commented Mar 12, 2025

marcellamaki commented Mar 12, 2025

marcellamaki left a comment

Choose a reason for hiding this comment

AllanOXDi commented Feb 14, 2025 •

edited

Loading

github-actions bot commented Feb 14, 2025 •

edited

Loading