Basic layout for displaying "resources currently in lesson" #12845

AllanOXDi · 2024-11-14T20:03:47Z

Summary

This PR implements the "shopping cart" preview functionality as part of aligning the lesson management workflow with the side panel management

References

closes #12787

Reviewer guidance

For now test if

It displays an array of contentnodes, all of which are resources (no folders)
In the bottom bar a link should be displayed whenever the array length is > 0
On "select" it opens "shopping cart"
The header should display the number of items in the selection array
The lesson name is rendered in
The "back" arrow "minimizes" the shopping cart view. The "X" closes the side panel. no validation for closing is required
If each resource in the array is displayed as an item
If each item should have a content node icon

Not working.
Displaying the file size for each item in the list.

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-21T20:08:53Z

Build Artifacts

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

AllanOXDi · 2024-11-22T19:04:47Z

hey @nucleogenesis this is the first part of the layout for displaying "resources currently in lesson. I have left few requirements out will get back to it on monday

marcellamaki

Hi @AllanOXDi. A lot of the foundational pieces are here! overall, I think the display of each item is visually close to done, and some of the core interactions on the side panel seem to be working. I'll leave additional feedback tomorrow, but I wanted to make sure I had done a preliminary review so that you had something to work on before I'm online.

my main question is I'm wondering why the planLessonRoutes file has been added back in here. Since Alex's finalization of the navigation updates and the refactoring there, it's no longer needed. Not sure if this just wasn't clear from the other project updates, or if this was a side effect of the rebase that you did and just got committed accidentally.

Other than that, I've left some inline comments, and I'll do a second pass tomorrow. Thank you! :)

marcellamaki · 2024-11-25T22:36:14Z

...coach/assets/src/views/lessons/LessonSummaryPage/sidePanels/ManageSelectedLessonResource.vue

+        icon="back"
+        @click="$router.go(-1)"
+      />
+      <span


We may refactor this a bit when we update the side panel overall, but this should be an h1 not a span. You may need to override some of the styles so it still is the same size, but it's important that semantically it's an h1

marcellamaki · 2024-11-25T22:39:06Z

...coach/assets/src/views/lessons/LessonSummaryPage/sidePanels/ManageSelectedLessonResource.vue

+      <div class="bottom-buttons-style">
+        <KButton
+          :primary="true"
+          text="save & finish"


Unwrapped string!

If this string already exists, let's use that one. If it doesn't, please add it to the searchAndFilters strings

marcellamaki · 2024-11-25T22:39:38Z

...coach/assets/src/views/lessons/LessonSummaryPage/sidePanels/ManageSelectedLessonResource.vue

+    },
+    mounted() {
+      setTimeout(() => {
+        console.log(this.currentLesson);


was this just here for testing?

marcellamaki · 2024-11-25T22:40:59Z

...bri/plugins/coach/assets/src/views/lessons/LessonSummaryPage/sidePanels/SelectedResource.vue

+      :key="index"
+    >
+      <KGrid :style="lessonOrderListButtonBorder">
+        <KGridItem :layout12="{ span: 6 }">


Can you update the KGridItems so that this aligns better on mobile? right now it's stacking oddly.

marcellamaki · 2024-11-25T22:41:42Z

...bri/plugins/coach/assets/src/views/lessons/LessonSummaryPage/sidePanels/SelectedResource.vue

+            <div>
+              <DragSortWidget
+                moveUpText="up"
+                moveDownText="down"


once you merge #12817, you'll need to update these^. you are now an expert on the fact that these need to be translated strings to use for the aria labels :)

marcellamaki · 2024-11-25T22:52:54Z

...bri/plugins/coach/assets/src/views/lessons/LessonSummaryPage/sidePanels/SelectedResource.vue

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


This is a bit of a nitpick, but I think calling this SelectedResources is more accurate than SelectedResource - to me the plural indicates the list, and the singular indicates the one item in the list

marcellamaki · 2024-11-25T23:04:02Z

...bri/plugins/coach/assets/src/views/lessons/LessonSummaryPage/sidePanels/SelectedResource.vue

+    mounted() {
+      setTimeout(() => {
+        this.resources = this.resourceList;
+      }, 1000);


This is causing you to have an "extra" loading state, and it's mismatched with your loading spinner condition. I'm not sure if this is also here for testing, or if there is something else happening with loading/state management that you're trying to work around, but if that's the case, let's chat more about the situation and see if we can find another solution

on a second read, I think perhaps what is happening is that you are making a copy of the resource list to test the "remove" functionality. Let's talk about this more, because we'll need a slightly different approach, based on the resources that will ultimately be passed in here (the current selection, as opposed to the current state of the lesson)

nucleogenesis

Left a few pieces of feedback, great work so far Allan!

nucleogenesis · 2024-11-27T22:47:17Z

...coach/assets/src/views/lessons/LessonSummaryPage/sidePanels/ManageSelectedLessonResource.vue

+    },
+    methods: {
+      closeSidePanel() {
+        this.$router.go(-2);


This should probably just go straight to the root page rather than relying on the specific configuration in the browser history.

For example, if you refreshed while on this page and closed the side panel, you might not get the behavior you expect here.

I see this is called when the user clicks "Save & Finish" - should this be updating something somewhere?

...bri/plugins/coach/assets/src/views/lessons/LessonSummaryPage/sidePanels/SelectedResource.vue

nucleogenesis

Some notes from review. Some of these things seem related to how things are being routed so I think that it might be worth chatting w/ @AlexVelezLl and looking at #12895 where he's working on the resource selection.

I mention this because I wonder if any changes there to how resources are selected altogether, so how you handle things like the bottom bar link respond to the user's choices may be affected by Alex's work.

Back arrow & side panel layout

When I am in the "Manage resources" page and open the "Shopping Cart" side panel, the underlying page is redirected back to the LessonRootPage somehow - so when I click the back arrow OR the X, I find myself at the list of all of my lessons.

Additionally, when I Save & Finish, I am not returned to the Lesson itself like I'd expect.

Bottom bar link

Shows when the value is 0
Does not update when I change my selection

Changes applied immediately, No "Are you sure?" confirmation modal when leaving without saving

When I remove an item and click the X to close the panel, I should be given an "Are you sure?" modal similar to what you'll see in Quiz Section Editor.

nucleogenesis · 2024-12-18T19:03:51Z

...ri/plugins/coach/assets/src/views/lessons/LessonSummaryPage/sidePanels/SelectedResources.vue

+      },
+    },
+
+    computed: {


Could use the themeToken fineLine here https://design-system.learningequality.org/colors/#tokens-fineLine

AllanOXDi · 2024-12-18T19:02:35Z

...coach/assets/src/views/lessons/LessonSummaryPage/sidePanels/ManageSelectedLessonResource.vue

+      setTimeout(() => {
+        this.getResources();
+      }, 2000);
+    },


This will be taken off- was for testing
The reason for the delay was to enable the component load fully with data, otherwise the function was returning undefined.

AllanOXDi · 2024-12-18T19:20:29Z

...coach/assets/src/views/lessons/LessonSummaryPage/sidePanels/ManageSelectedLessonResource.vue

+      };
+    },
+    computed: {
+      ...mapState('lessonSummary', ['currentLesson', 'workingResources', 'resourceCache']),


This will be removed in favor of a composable

AllanOXDi · 2024-12-18T19:23:20Z

...coach/assets/src/views/lessons/LessonSummaryPage/sidePanels/ManageSelectedLessonResource.vue

+            this.getLearnersForLesson(this.currentLesson),
+          );
+
+          const tableRow = {


This will also be replaced by useResourceselection compasable. We will not need it anymore

marcellamaki

Okay @AllanOXDi - I think this is ready to go! Please remove any code that will not be used (such as what you reference with the setTimeout, etc. that was just for testing). Once you do that, we will be able to merge. I'll open a follow up issue about the next steps once we get this and Alex's work into develop! Nice work

AllanOXDi · 2024-12-19T15:06:21Z

Okay! thanks

marcellamaki

Thank you, @AllanOXDi!

Discussed requested changes with Jacob - some addressed, some will be follow up

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

AllanOXDi changed the title ~~WIP- don't even bother at it.~~ WIP- don't even bother looking at it. Nov 14, 2024

AllanOXDi mentioned this pull request Nov 19, 2024

Lesson Summary children routes refactor #12852

Merged

AllanOXDi force-pushed the basic-layout-for-lesson-resources branch from 64c274c to e18471f Compare November 21, 2024 19:37

AllanOXDi force-pushed the basic-layout-for-lesson-resources branch from 38f90a6 to 4fa945b Compare November 22, 2024 18:03

AllanOXDi changed the title ~~WIP- don't even bother looking at it.~~ Basic layout display for "resources currently in lesson" Nov 22, 2024

AllanOXDi requested review from nucleogenesis and marcellamaki November 22, 2024 19:02

AllanOXDi marked this pull request as ready for review November 22, 2024 19:02

AllanOXDi changed the title ~~Basic layout display for "resources currently in lesson"~~ Basic layout for displaying "resources currently in lesson" Nov 22, 2024

marcellamaki requested changes Nov 25, 2024

View reviewed changes

nucleogenesis requested changes Nov 27, 2024

View reviewed changes

AllanOXDi force-pushed the basic-layout-for-lesson-resources branch 4 times, most recently from e888e12 to d1cfe13 Compare December 5, 2024 17:56

AllanOXDi force-pushed the basic-layout-for-lesson-resources branch 3 times, most recently from 515574c to b2fdfa0 Compare December 5, 2024 18:50

AllanOXDi requested review from nucleogenesis and marcellamaki December 11, 2024 14:19

nucleogenesis previously requested changes Dec 16, 2024

View reviewed changes

AllanOXDi added 5 commits December 17, 2024 18:28

basic layout added

95d5985

fixes routes

7291c44

fixes the routing issue

4e571ab

clean up routing business

0b1e4a0

clean ups importations

c5a0cce

AllanOXDi and others added 16 commits December 17, 2024 18:28

clean timout

257bd3f

rebase

5264a6c

fix review comments

76a4fd7

adds a draggable

adaf345

removed unused variables

620e709

removes unused data prop

9bd9be7

[pre-commit.ci lite] apply automatic fixes

a3cb994

handles navigation

25cafe4

fixes failing test

2cb92ed

fixes failing test

e925502

fixes failing test

5ff97a3

updates the test

6fef976

restores test

8bc13de

fixes comments

01d182e

fixes comments

a9eb601

more comment fix

26142c6

AllanOXDi force-pushed the basic-layout-for-lesson-resources branch from 15c17e9 to 26142c6 Compare December 18, 2024 18:58

nucleogenesis reviewed Dec 18, 2024

View reviewed changes

fix failing check

d4fe9f5

AllanOXDi commented Dec 18, 2024

View reviewed changes

themetoken

12e2a32

marcellamaki reviewed Dec 19, 2024

View reviewed changes

clean up code

2218fbb

AllanOXDi requested review from marcellamaki and nucleogenesis December 19, 2024 18:21

marcellamaki approved these changes Dec 19, 2024

View reviewed changes

marcellamaki merged commit af1eedb into learningequality:develop Dec 19, 2024
37 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Basic layout for displaying "resources currently in lesson" #12845

Basic layout for displaying "resources currently in lesson" #12845

AllanOXDi commented Nov 14, 2024 •

edited

Loading

github-actions bot commented Nov 21, 2024 •

edited

Loading

AllanOXDi commented Nov 22, 2024 •

edited

Loading

marcellamaki left a comment

marcellamaki Nov 25, 2024

marcellamaki Nov 25, 2024

marcellamaki Nov 25, 2024

marcellamaki Nov 25, 2024

marcellamaki Nov 25, 2024 •

edited

Loading

marcellamaki Nov 25, 2024

marcellamaki Nov 25, 2024

marcellamaki Nov 25, 2024

nucleogenesis left a comment

nucleogenesis Nov 27, 2024

nucleogenesis Nov 27, 2024

nucleogenesis left a comment

nucleogenesis Dec 18, 2024

AllanOXDi Dec 18, 2024

AllanOXDi Dec 18, 2024

AllanOXDi Dec 18, 2024

marcellamaki left a comment

AllanOXDi commented Dec 19, 2024

marcellamaki left a comment

Basic layout for displaying "resources currently in lesson" #12845

Basic layout for displaying "resources currently in lesson" #12845

Conversation

AllanOXDi commented Nov 14, 2024 • edited Loading

Summary

References

Reviewer guidance

Testing checklist

PR process

Reviewer checklist

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

AllanOXDi commented Nov 22, 2024 • edited Loading

marcellamaki 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

marcellamaki Nov 25, 2024 • 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

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

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

AllanOXDi commented Dec 19, 2024

marcellamaki left a comment

Choose a reason for hiding this comment

AllanOXDi commented Nov 14, 2024 •

edited

Loading

github-actions bot commented Nov 21, 2024 •

edited

Loading

AllanOXDi commented Nov 22, 2024 •

edited

Loading

marcellamaki Nov 25, 2024 •

edited

Loading