Add quiz recipients selector as Side Panel #12952

AlexVelezLl · 2024-12-16T19:20:50Z

Summary

Add quiz recipients selector as Side Panel.

Compartir.pantalla.-.2024-12-17.10_59_21.mp4

References

Closes #12901.

…

Reviewer guidance

Create a new quiz and assign an entire class, groups or students.
Edit a created quiz and assig an entire class, groups or students.
Check error statuses.

github-actions · 2024-12-16T19:54:58Z

Build Artifacts

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

AlexVelezLl · 2024-12-17T16:36:58Z

kolibri/plugins/coach/assets/src/views/common/assignments/IndividualLearnerSelector/index.vue

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


This is just a refactor to break down the current IndividualLearnerSelector component to have an IndividualLearnerSelectorTable that we can reuse in the LearnersSelectorSidePanel. I havent edited the logic of this component, just extracted the table.

AlexVelezLl · 2024-12-17T16:39:23Z

...ssets/src/views/common/assignments/SidePanelRecipientsSelector/LearnersSelectorSidePanel.vue

+        {{ $tr('selectGroupsAndIndividualLearnersTitle') }}
+      </h1>
+    </template>
+    <div style="padding-top: 30px">


Once #12895 is merged we will need to remove this div, as its adding a fixing-padding that wont be needed after #12895

AlexVelezLl · 2024-12-17T16:39:40Z

...ssets/src/views/common/assignments/SidePanelRecipientsSelector/LearnersSelectorSidePanel.vue

+    display: flex;
+    justify-content: flex-end;
+    width: 100%;
+    margin-bottom: 16px;


Once #12895 is merged we will need to remove this maring, as its adding a fixing-marging that wont be needed after #12895

marcellamaki

Overall this looks really good @AlexVelezLl - I don't think I'll have time to do a second pass today (so it will be merged after the new year) but on initial read through overall the code changes make sense to me. I'd love for @pcenov to do some more robust QA especially since he is so good at finding those edge cases. If you can get to it tomorrow (Friday 20th) Peter, great, but no problem if it's not til after the holidays as well.

One small UI note - I'm not sure if this may be resolved after we merge your resource selection and sidepanel refactors PR, but the page content for a long learner list is cut off. All of the learners are visible, but because the pagination is covered by the bottom bar, it looks as if something might be "missing" (and presumably, if there were enough learners to paginate, this would prevent the user from being able to). If this will be fixed after rebasing that other PR, please disregard!

after deleting the bottom bar element in the browser:

marcellamaki · 2024-12-19T16:53:50Z

kolibri/plugins/coach/assets/src/views/common/assignments/AssignmentDetailsModal.vue

@@ -11,6 +11,10 @@
        {{ submitErrorMessage }}
      </UiAlert>

+      <!--
+        TODO: Refactor or rename this component, setting a title or a description
+        is not part of an "assignment" process (?)


Yes, I think calling it something more like "QuizTitleAndRecipientsHeader" would make more sense even if it's longer

marcellamaki · 2024-12-19T17:25:06Z

...ts/src/views/common/assignments/IndividualLearnerSelector/IndividualLearnerSelectorTable.vue

+    @pageChanged="currentPageLearners = $event.items"
+  >
+    <template #default="{ items }">
+      <CoreTable


This is absolutely not required and not blocking, but just a thought, @AlexVelezLl - do you think that using the new KTable here would be difficult to refactor in? If it's going to add lots of work, we don't have to. I just wonder if there might be any a11y benefits and this is a moment when we could refactor it in "in the course of" rather than in separate work as tech debt tasks.

Maybe you could spend 30-60 minutes and see how it goes? If it seems like it would take a lot longer and introduce complexities, we don't have to do it now. If it seems like something you could do in a few hours, perhaps worth it (especially as the next couple of weeks will be quite)

For sure! I can give it a try and see how it goes :).

marcellamaki · 2024-12-19T18:25:12Z

kolibri/plugins/coach/assets/src/views/quizzes/QuizSummaryPage/QuizOptionsDropdownMenu.vue

          {
            label: this.$tr('copyQuizAction'),
            value: 'COPY',
          },
          { label: this.coreString('deleteAction'), value: 'DELETE' },
        ];
+        if (!this.exam?.archive) {


Is this just because the archive key would exist so we can use that instead of the draft prop? If so, how is that distinct from this.exam?.draft that we are using below for the label condition?

Oh no, there are three different "states". exam in draft, exam "active"(once the exam is published) and exam closed. The this.exam?.archive represents that the exam was closed. So when the exam is "not archived", it can be either in draft or not.

AlexVelezLl · 2024-12-19T19:03:43Z

Thanks @marcellamaki. Yes! The issue with the hidden pagination buttons will be fixed once #12895 is merged too :).

pcenov · 2024-12-20T12:55:57Z

Hi @AlexVelezLl and @marcellamaki, just some comments/points for discussion:

Why are we introducing the "All Ungrouped Learners" label? Previously we had "Individual learners" and it's still in use in Coach > Lessons. If we decide to use the new label, then the capitalization should be corrected, so that it's changed to "All ungrouped learners", but also we will have to use the same label at Coach > Lessons for consistency.

With the current implementation of the "All ungrouped learners" checkbox it's duplicating the function of the "Select all on page" checkbox in the '"Individual learners table. Previously selecting the "Individual learners" button resulted in actually showing the "Individual learners" table while now it's constantly shown. So again the question is - will that be implemented in the same way in Coach > Lessons and is that the correct implementation?

select.all.mp4

Also when there aren't any learners yet the checkbox "All ungrouped learners" is selected by default and cannot be deselected:

Introducing validation here is also a bit problematic, because currently I can create and assign a quiz to the entire class when there's not a single learner enrolled in that class, I can also assign the quiz to an empty group but if I select the "Groups and individual learners" radio button and don't select learners then I am seeing the validation. It even let's me "save" the side panel selection without any users selected:

validation.mp4

pcenov · 2024-12-20T13:08:54Z

Also we probably should have the same implementation when copying a quiz, as currently it's not changed there:

copy.quiz.mp4

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

AlexVelezLl marked this pull request as ready for review December 17, 2024 15:54

AlexVelezLl commented Dec 17, 2024

View reviewed changes

AlexVelezLl added this to the Kolibri 0.18: Coach Resource Discovery milestone Dec 18, 2024

marcellamaki self-assigned this Dec 19, 2024

marcellamaki reviewed Dec 19, 2024

View reviewed changes

AlexVelezLl added 8 commits December 19, 2024 15:04

Add initial implementation of side panel recipients selector

0ffd2b5

Manage save button, show recipients list

97530d8

Manage errors and alerts

603351d

Manage edit quizzes and show server error when validation is invalid

425c503

Validate exam ended to show edit quiz

ec7f747

Fix linting

c6f7453

Match styles to specs

0e98688

Fix styles

ef6d377

AlexVelezLl force-pushed the quiz-assignment-side-panel branch from bb801dd to ef6d377 Compare December 19, 2024 20:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add quiz recipients selector as Side Panel #12952

Add quiz recipients selector as Side Panel #12952

AlexVelezLl commented Dec 16, 2024 •

edited

Loading

github-actions bot commented Dec 16, 2024 •

edited

Loading

AlexVelezLl Dec 17, 2024 •

edited

Loading

AlexVelezLl Dec 17, 2024

AlexVelezLl Dec 17, 2024

marcellamaki left a comment

marcellamaki Dec 19, 2024

marcellamaki Dec 19, 2024

marcellamaki Dec 19, 2024

AlexVelezLl Dec 19, 2024

marcellamaki Dec 19, 2024

AlexVelezLl Dec 19, 2024

AlexVelezLl commented Dec 19, 2024 •

edited

Loading

pcenov commented Dec 20, 2024 •

edited

Loading

pcenov commented Dec 20, 2024

Add quiz recipients selector as Side Panel #12952

Are you sure you want to change the base?

Add quiz recipients selector as Side Panel #12952

Conversation

AlexVelezLl commented Dec 16, 2024 • edited Loading

Summary

References

Reviewer guidance

github-actions bot commented Dec 16, 2024 • edited Loading

AlexVelezLl Dec 17, 2024 • edited Loading

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

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 commented Dec 19, 2024 • edited Loading

pcenov commented Dec 20, 2024 • edited Loading

pcenov commented Dec 20, 2024

AlexVelezLl commented Dec 16, 2024 •

edited

Loading

github-actions bot commented Dec 16, 2024 •

edited

Loading

AlexVelezLl Dec 17, 2024 •

edited

Loading

AlexVelezLl commented Dec 19, 2024 •

edited

Loading

pcenov commented Dec 20, 2024 •

edited

Loading