Adds ability to preview non-practice resources from the sidepanel #13012

AllanOXDi · 2025-01-17T19:02:52Z

Summary

Updated the KCard in the resource selection in lessons to navigate to preview resources.
Used the existing content renderer to display the content using the /lessonstemp/ path in the updated side panel.

Before

After

Closes #12988

References

#12988

Reviewer guidance

See this
Navigate through the lesson workflow using the /lessonstemp path.
Can you successfully preview any non-exercise materials that use the content renderer ?
Is the side panel mobile responsiveness and a11y?

Currently not implemented

Ability to add a resource to a lesson
Have not figured out what triggered the KBreadcrumbs error in the console

…

github-actions · 2025-01-17T19:41:08Z

Build Artifacts

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

AlexVelezLl

Hey @AllanOXDi! I have left just a few comments, mainly to migrate this work to the new subPages structure we have for the LessonResourceSelection side panel :). Please let me know if you have any question :)

AlexVelezLl · 2025-01-22T14:05:20Z

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

@@ -155,18 +155,24 @@ export default [
            path: 'channels',
            component: SelectFromChannels,
          },
+          {
+            name: PageNames.LESSON_PREVIEW_RESOURCE,
+            path: 'preview-selected-resources',


I think the path here could be just "preview" or "preview-resource" as "preview-selected-resources" is not completely acurate and can be confused with path to the shopping cart

I see that previous to this change, we had a path like this preview-resources/:nodeId:

{ name: PageNames.LESSON_PREVIEW_RESOURCE, path: 'preview-resources/:nodeId', component: PreviewSelectedResources, },

Was there any reason to change the path and the param to be a query?

I don't think there was a particular reason to

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

kolibri/plugins/coach/assets/src/views/lessons/LessonSummaryPage/sidePanels/PreviewContent.vue

AlexVelezLl · 2025-01-22T15:01:42Z

kolibri/plugins/coach/assets/src/views/lessons/LessonSummaryPage/sidePanels/PreviewContent.vue

+
+  <div>
+    <KGrid>
+      <KGridItem


We also have this in the SelectFromChannels view. See how you can do this with just a few lines using flex: https://github.com/allanoxdi/kolibri/blob/dbae95ec4f53687239997acd8f8278f555c0507e/kolibri/plugins/coach/assets/src/views/lessons/LessonSummaryPage/sidePanels/LessonResourceSelection/subPages/SelectFromChannels.vue#L4

Hey @AllanOXDi! Just checking in if you looked at the flex example :). If you have strong opinions to use KGrid here lets chat!

Also, just noticed that its a little bit misaligned 😅

kolibri/plugins/coach/assets/src/views/lessons/LessonSummaryPage/sidePanels/PreviewContent.vue

nucleogenesis

I'll do some manual testing to follow-up, but have left a few code comments & questions.

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

nucleogenesis · 2025-01-27T20:24:36Z

kolibri/plugins/coach/assets/src/views/lessons/LessonSummaryPage/UpdatedResourceSelection.vue

+          params: {
+            ...params,
+          },


Non-blocking, this could just be simplified to params, since you're not modifying anything

kolibri/plugins/coach/assets/src/views/lessons/LessonSummaryPage/UpdatedResourceSelection.vue

AlexVelezLl

Thanks @AllanOXDi! This is looking good!

I have just commented a few things more of the files structure :). And while doing manual QA I found that the breadcrumbs links are not working properly, these should redirect to the folder within the same side panel:

Compartir.pantalla.-.2025-01-27.17_32_17.mp4

Also, it seems that the add button is currently not working:

Compartir.pantalla.-.2025-01-27.17_48_11.mp4

AlexVelezLl · 2025-01-27T22:14:43Z

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

+    }).then(node => {
+      contentNode.value = node;
+      if (node != null) {
+        loading.value = false;


If it is null then will loading be true indefinitely?

Yes, I see! if node is null, theloading.valuewill remain true indefinitely because the condition to setloading.value = falseis only executed whennodeis not null. I can can see an infinite loading state here. Thanks for pointing that out

I'm not sure node can ever be null here - I don't think our api resource methods like fetchModel would return anything besides the data it gets from the API request or it throws an error.

So, in this case, I think you'd probably want to just treat your then() callback function as if it's exactly what you expected to get.

Then you can chain a catch method on it - but you'll need a way to tell the caller of useFetchContentNode to tell the Vue component it is used in that there were errors.

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

kolibri/plugins/coach/assets/src/views/lessons/LessonSummaryPage/sidePanels/PreviewContent.vue

AlexVelezLl · 2025-01-27T22:27:29Z

kolibri/plugins/coach/assets/src/views/lessons/LessonSummaryPage/sidePanels/PreviewContent.vue

+
+  <div>
+    <KGrid>
+      <KGridItem


Hey @AllanOXDi! Just checking in if you looked at the flex example :). If you have strong opinions to use KGrid here lets chat!

AlexVelezLl · 2025-01-27T22:35:50Z

kolibri/plugins/coach/assets/src/views/lessons/LessonSummaryPage/sidePanels/PreviewContent.vue

+
+  <div>
+    <KGrid>
+      <KGridItem


Also, just noticed that its a little bit misaligned 😅

...s/LessonSummaryPage/sidePanels/LessonResourceSelection/subPages/PreviewSelectedResources.vue

nucleogenesis · 2025-01-29T17:01:30Z

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

+    });
+  };
+
+  fetchContentNode();


Non-blocking note. You could skip making the function fetchContentNode altogether and just run the code within it directly since you're not returning the function from the composable and you call it within the comopsable.

nucleogenesis

No way back from preview screen

I think this needs a back-arrow & handler

Cannot preview Exercise content type

Seems like this might be that this.questions is undefined in the selectedQuestion computed property.

Otherwise, this is looking great! I tested adding/removing and going back/forth through the navigation and things are looking & feeling great.

AllanOXDi · 2025-01-30T17:32:20Z

No way back from preview screen
I think this needs a back-arrow & handler.
This is something I discussed with @marcellamaki after @AlexVelezLl comment here

AllanOXDi requested review from nucleogenesis, marcellamaki and AlexVelezLl January 17, 2025 19:02

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

AlexVelezLl requested changes Jan 22, 2025

View reviewed changes

AllanOXDi marked this pull request as ready for review January 24, 2025 15:40

AllanOXDi requested a review from AlexVelezLl January 24, 2025 15:40

nucleogenesis reviewed Jan 27, 2025

View reviewed changes

AlexVelezLl reviewed Jan 27, 2025

View reviewed changes

nucleogenesis self-assigned this Jan 28, 2025

AllanOXDi force-pushed the preview-resource branch from a23e1f2 to b5e50d9 Compare January 28, 2025 19:50

AllanOXDi requested review from AlexVelezLl and nucleogenesis January 28, 2025 19:53

nucleogenesis reviewed Jan 29, 2025

View reviewed changes

AllanOXDi added 13 commits January 30, 2025 15:18

adds ability to preview a resource content from the sidepanel

91bcdfe

more styling

20f56bd

breadcrumbs styling

829df8f

breadcrumbs styling

a108bd8

adds loading state

b213d93

use router-view to render the sidepanel

6a58403

use contentnodekinds constants

a2d12b6

improves loading state

7c5c5b4

re arranges the files in the subpaes folder

62169a7

use flex to align header items'

0ac4a8c

breadcrumbs link routes back to resource tree

d6073f9

renames a class

4a8a42f

adds resources to selected resources pool using add button

a5a0297

AllanOXDi force-pushed the preview-resource branch from b5e50d9 to a5a0297 Compare January 30, 2025 14:55

nucleogenesis requested changes Jan 30, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds ability to preview non-practice resources from the sidepanel #13012

Adds ability to preview non-practice resources from the sidepanel #13012

AllanOXDi commented Jan 17, 2025

github-actions bot commented Jan 17, 2025 •

edited

Loading

AlexVelezLl left a comment

AlexVelezLl Jan 22, 2025

AlexVelezLl Jan 22, 2025

AllanOXDi Jan 22, 2025

AlexVelezLl Jan 22, 2025

AlexVelezLl Jan 27, 2025

AlexVelezLl Jan 27, 2025

nucleogenesis left a comment

nucleogenesis Jan 27, 2025

AlexVelezLl left a comment

AlexVelezLl Jan 27, 2025

AllanOXDi Jan 28, 2025

nucleogenesis Jan 29, 2025 •

edited

Loading

AlexVelezLl Jan 27, 2025

AlexVelezLl Jan 27, 2025

nucleogenesis Jan 29, 2025

nucleogenesis left a comment

AllanOXDi commented Jan 30, 2025

Adds ability to preview non-practice resources from the sidepanel #13012

Are you sure you want to change the base?

Adds ability to preview non-practice resources from the sidepanel #13012

Conversation

AllanOXDi commented Jan 17, 2025

Summary

Before

After

References

Reviewer guidance

Currently not implemented

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

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

nucleogenesis left a 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

nucleogenesis Jan 29, 2025 • edited Loading

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

nucleogenesis left a comment

Choose a reason for hiding this comment

AllanOXDi commented Jan 30, 2025

github-actions bot commented Jan 17, 2025 •

edited

Loading

nucleogenesis Jan 29, 2025 •

edited

Loading