Integrate search resource selection #13008

AlexVelezLl · 2025-01-15T22:45:52Z

Summary

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

Compartir.pantalla.-.2025-01-15.17_44_57.mp4

References

Closes #12987

Reviewer guidance

Edit a lesson
Change browser route path from "/lesson" to "lessontemp".
Click on "Manage resources".
Click on the search 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, browser back/forward buttons navigation, etc.

github-actions · 2025-01-16T15:39:47Z

Build Artifacts

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

marcellamaki · 2025-01-16T16:17:29Z

@pcenov - this is not urgent, but depending on how time consuming your laptop reset process is, this could go to the top of your list for Friday or Monday. Thank you!

pcenov · 2025-01-20T14:27:43Z

Hi @AlexVelezLl and @marcellamaki - I confirm that the search is integrated as specified above.

Here are a few general observations based on my testing which can probably be tracked as follow-up issues if not already tracked somewhere else:

We probably need to indicate in a better way within the search panel that there are no channels to import from when there aren't any as currently this is what I see in that case:

As specified in Figma, when we open the search panel the Category filter should be expanded by default while currently it's not:

3 There are quite a few design differences if the current implementation is closely compared to Figma such as the back arrow and the label are not centered, the keywords label should be removed, the icons for the categories are not added etc. Since I am not sure if this is in scope for this PR I am just mentioning it here as a consideration:

When searching by selecting filters it's not immediately clear how the user should proceed the search (the user has to click the search icon next to the empty keywords field). In this case I was expecting to immediately see the filtered results after having applied a filter:

not.clear.how.to.proceed.mp4

When testing the mobile view in Chrome's mobile devices emulator everything looks ok, but when I open the search panel in Chrome on my Samsung Note 10 phone I can only see the 'N resources selected' text and not the "Save & Finish" button as it's covered by the device's taskbar:

nucleogenesis

Overall the changes are looking good. I left a handful of largely non-blocking comments. Happy to chat further about anything I noted.

I haven't finished going through every file yet, so I will be coming back to give a few parts another closer look

kolibri/plugins/coach/assets/src/composables/useResourceSelection.js

nucleogenesis · 2025-01-20T20:53:09Z

...ugins/coach/assets/src/views/lessons/LessonResourceSelectionPage/LessonContentCard/index.vue

    },
    computed: {
      isTopic() {
        return !this.content.isLeaf;
      },
+      headingElement() {
+        return this.headingLevel ? `h${this.headingLevel}` : 'h3';


Non-blocking: given the default value of headingLevel the conditional could be removed here.

...ugins/coach/assets/src/views/lessons/LessonResourceSelectionPage/LessonContentCard/index.vue

AlexVelezLl · 2025-01-21T14:48:18Z

Thanks @pcenov!

I think this can be reported in a different issue :).
Just implemed this!
I have added the "Search in ..." as title on top of the search box when we open the filters from within a foder, and "Keywords" if we open it from the index (where we see the channels and bookmarks).

I think the icons are still not available cc: @marcellamaki, so probably should be reported in a different issue. And the border-bottom in the side panel modal header I think its something we have in develop but not in the figma specs. (honestly I am not sure neither about the reason of this difference but it has been accepted for a long time 😅 cc: @marcellamaki)

@marcellamaki and @jtamiace will tell better about the expected behaviour of this. Right now the current implementation is that the user needs to click on the search button to open the search results.
Im not being able to replicate this, I have tried it on my browser but it seems the line is breaking correctly. Could you provide more details how can I reproduce it?

pcenov · 2025-01-22T15:23:08Z

Thank @AlexVelezLl,
Here are my comments for the issues we are discussing:

I have reported a follow-up issue here Manage lesson resources > Search - No indication that there are no imported resources on the device #13021
I confirm that this is fixed now.
For points 3, 4 @marcellamaki pls let us know how to proceed. A note that in the Library page we are showing the results to the user immediately after applying the filter so that's my expectation for this search too. :)
@AlexVelezLl in order to replicate the issue you need to check it in an actual mobile phone as the problem is caused by the device's taskbar. I have double checked that this happens consistently. :)

AlexVelezLl · 2025-01-27T13:59:07Z

Hi @pcenov! I have pushed some changes to open the search results just after clicking on a filter. Also I think I have fixed the wrong positioning of the bottom bar when we open the side panel in a mobile device, could you give it a check, please? 😃

pcenov · 2025-01-28T13:52:18Z

Thanks @AlexVelezLl - I confirm that now selecting a filter bring the user to the search results and that the "Save & Finish" button is no longer covered by the device's taskbar.

I was able to identify one additional mobile issue - the filter by categories is not working in mobile view. This can be replicated both in Chrome's emulator and in an actual device:

cannot.select.a.category.in.mobile.view.mp4

As always, let me know if this is not in scope for this PR and I'll file a follow-up issue.

marcellamaki · 2025-01-28T14:14:15Z

@pcenov @AlexVelezLl - I wonder if this Categories bug on mobile is related to the other one (which I don't remember where it popped up, actually.... last week). But it was I think a discussion of the windowIsLarge view and the conditions of the category search modal that had been moved to common. If it is in fact that, I think we can open a follow up issue (or add this to an existing issue), @AlexVelezLl. Sorry my brain is not 100% back online yet 😄 And thank you Alex for adjusting the expected behavior for the search results!

AlexVelezLl · 2025-01-28T14:18:27Z

I wonder if this Categories bug on mobile is related to the other one (which I don't remember where it popped up, actually.... last week). But it was I think a discussion of the windowIsLarge

Yes @marcellamaki, its the same, once that one is fixed, this will be fixed automatically :). I think we didnt end up with any follow up actions in that conversation 😅.

marcellamaki · 2025-01-28T14:31:27Z

Ah okay - I will open an issue then and we can put it in the backlog for the meeting today. Thank you for remembering Alex :)

marcellamaki · 2025-01-29T16:11:47Z

And back to answer questions on a few other points:

I also do not know why there is a bottom border on the header. I think we can remove it, but it doesn't have to be in the issue, if you'd like to file a follow up @pcenov.
the icons we will need to add in KDS, and then update in Kolibri, so I will open issues for that as well. I lost track of this and appreciate the reminder

I think that is it on the remaining questions that you both had? So, both are important but neither blocking for this PR. And, I am in process of re-reviewing this PR for code review and will add any additional code comments in a little while :)

marcellamaki

Hi @AlexVelezLl - I've added a few comments, I don't think any of them are blocking but more just thoughts and points of discussion. I'll do one more re-review, but then I think we will probably be able to go ahead with approving and merging this!

...ugins/coach/assets/src/views/lessons/LessonResourceSelectionPage/LessonContentCard/index.vue

...oach/assets/src/views/lessons/LessonSummaryPage/sidePanels/LessonResourceSelection/index.vue

marcellamaki · 2025-01-30T15:56:28Z

packages/kolibri-common/components/SearchChips.vue

@@ -13,7 +13,7 @@
          icon="delete"
          size="mini"
          class="filter-chip-button"
-          @click="$emit('removeItem', item)"
+          @click="$emit('removeItem', item, { isLast: items.length === 1 })"


is this to indicate that there are no more search chips and so in addition to removing the item we want to.... change something else about the UI?

Yes, I did it to save us a tick from redirecting back. The main "thing" here is that searchTerms gets its value directly from the route query, that means that we need the route query to be updated to have its value updated, so it will "cost" a couple of ticks more before we have the displayingSearchResults prop to be false and then to redirect.

An alternative could be just to watch displayingSearchResults and wait until it is false to do the redirectBack, but wanted to be more proactive, save a couple of ticks (I mean, not waiting until displayingSearchResults gets updated) and be explicit in the code that the last item being removed will redirect us back.

packages/kolibri-common/components/SearchFiltersPanel/index.vue

packages/kolibri-common/components/accordion/AccordionContainer.vue

marcellamaki · 2025-01-31T19:21:01Z

I've done a little bit more manual QA, and I noticed what seems like a rather specific edge case, but searching at the "all channels" level, if I navigate into a folder within the search, the "back to search results" returns me to the main seleciton index, not the search results. (This isn't the case with search results that are within a channel or folder)

Screen.Recording.2025-01-31.at.1.51.16.PM.mov

AlexVelezLl · 2025-02-01T00:42:41Z

Thanks @marcellamaki! I hadnt realized that case existed, I have just fixed it, and rebased my PR :)

ozer550 · 2025-02-03T06:03:53Z

kolibri/plugins/coach/assets/src/composables/useResourceSelection.js

+  const searchFetch = {
+    data: useSearchObject.results,
+    loading: useSearchObject.searchLoading,
+    hasMore: computed(() => !!useSearchObject.more.value),


Can this be reduced just to hasMore: computed(() => useSearchObject.more.value) ?

Hi @ozer550! In this case its not the same, because hasMore should be a boolean value, and useSearchObject.more.value is an object, the "more" object. So to transform it to boolean we can do either !!useSearchObject.more.value or Boolean(useSearchObject.more.value), but personally I like more the former 😅.

marcellamaki

Thank you @AlexVelezLl - I think all feedback has been addressed now. While I suspect Peter and Radina may find a few more things when we do full QA of this feature, I think this is in good shape to merge, and we can handle any follow up issues as they come. 🎉

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

AlexVelezLl requested review from marcellamaki, nucleogenesis and rtibbles January 15, 2025 22:46

AlexVelezLl force-pushed the integrate-search-resource-selection branch from c731c48 to f982e74 Compare January 16, 2025 15:04

nucleogenesis reviewed Jan 20, 2025

View reviewed changes

pcenov mentioned this pull request Jan 22, 2025

Manage lesson resources > Search - No indication that there are no imported resources on the device #13021

Closed

AlexVelezLl force-pushed the integrate-search-resource-selection branch from 22dc161 to 26db2b8 Compare January 24, 2025 16:30

marcellamaki mentioned this pull request Jan 28, 2025

Fix bugs in common CategorySearchModal #13033

Closed

nucleogenesis assigned marcellamaki Jan 28, 2025

marcellamaki reviewed Jan 30, 2025

View reviewed changes

AlexVelezLl added 6 commits January 31, 2025 19:23

Fix SearchFilterPanel showing a blank div

04ffcbb

Integrate Search Filters and Search Results to LessonResourceSelection

fc945b4

Support browsing search result topics

3e8d6a5

Immersive side panel when browsing search result topics

ee35e76

Improve styles in search filters subpage

74857ee

Fixes

e960ff2

AlexVelezLl added 10 commits January 31, 2025 19:29

Refactor and add comments on some methods

6734671

Move strings to searchAndFilterStrings and fix styles

aed4dcc

Optimize search requests

32a75be

Allow single select mode in acordions and open by default item

41353ff

Add title and go back method when selecting a category

8d5f8bb

Align Search Filter Panel styles with figma

569dace

Fix styles

edf6801

Redirect to search results on filter change

65bc654

Fix height calculation of side panel in mobile devices

b936e25

Redirect to search results when searching from index page

696cfa9

AlexVelezLl force-pushed the integrate-search-resource-selection branch from e9575f3 to 696cfa9 Compare February 1, 2025 00:41

ozer550 reviewed Feb 3, 2025

View reviewed changes

marcellamaki approved these changes Feb 4, 2025

View reviewed changes

marcellamaki merged commit 4b993e4 into learningequality:develop Feb 4, 2025
37 checks passed

AlexVelezLl deleted the integrate-search-resource-selection branch February 4, 2025 14:27

marcellamaki mentioned this pull request Feb 4, 2025

Refactor ContentCardList to use the new KCardGrid and the AccessibleResourceCard and AccessibleFolderCard #12732

Open

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Integrate search resource selection #13008

Integrate search resource selection #13008

AlexVelezLl commented Jan 15, 2025 •

edited

Loading

github-actions bot commented Jan 16, 2025 •

edited

Loading

marcellamaki commented Jan 16, 2025

pcenov commented Jan 20, 2025

nucleogenesis left a comment

nucleogenesis Jan 20, 2025

AlexVelezLl Jan 21, 2025

AlexVelezLl commented Jan 21, 2025

pcenov commented Jan 22, 2025

AlexVelezLl commented Jan 27, 2025

pcenov commented Jan 28, 2025

marcellamaki commented Jan 28, 2025

AlexVelezLl commented Jan 28, 2025 •

edited

Loading

marcellamaki commented Jan 28, 2025

marcellamaki commented Jan 29, 2025

marcellamaki left a comment

marcellamaki Jan 30, 2025

AlexVelezLl Feb 1, 2025

marcellamaki commented Jan 31, 2025

AlexVelezLl commented Feb 1, 2025

ozer550 Feb 3, 2025

AlexVelezLl Feb 3, 2025

marcellamaki left a comment

Integrate search resource selection #13008

Integrate search resource selection #13008

Conversation

AlexVelezLl commented Jan 15, 2025 • edited Loading

Summary

References

Reviewer guidance

github-actions bot commented Jan 16, 2025 • edited Loading

Build Artifacts

marcellamaki commented Jan 16, 2025

pcenov commented Jan 20, 2025

nucleogenesis left a comment

Choose a reason for hiding this comment

nucleogenesis Jan 20, 2025

Choose a reason for hiding this comment

AlexVelezLl Jan 21, 2025

Choose a reason for hiding this comment

AlexVelezLl commented Jan 21, 2025

pcenov commented Jan 22, 2025

AlexVelezLl commented Jan 27, 2025

pcenov commented Jan 28, 2025

marcellamaki commented Jan 28, 2025

AlexVelezLl commented Jan 28, 2025 • edited Loading

marcellamaki commented Jan 28, 2025

marcellamaki commented Jan 29, 2025

marcellamaki left a comment

Choose a reason for hiding this comment

marcellamaki Jan 30, 2025

Choose a reason for hiding this comment

AlexVelezLl Feb 1, 2025

Choose a reason for hiding this comment

marcellamaki commented Jan 31, 2025

AlexVelezLl commented Feb 1, 2025

ozer550 Feb 3, 2025

Choose a reason for hiding this comment

AlexVelezLl Feb 3, 2025

Choose a reason for hiding this comment

marcellamaki left a comment

Choose a reason for hiding this comment

AlexVelezLl commented Jan 15, 2025 •

edited

Loading

github-actions bot commented Jan 16, 2025 •

edited

Loading

AlexVelezLl commented Jan 28, 2025 •

edited

Loading