0.18: Groups view updates #12696

nucleogenesis · 2024-10-02T23:03:54Z

Summary

2024-10-02.16-05-03.mp4

Updates strings to match new designs per Update views to create new "Groups" pages #12678
"Learn more" link opens new "About groups" modal
Adds "Learn more" to common strings
Adds "header" slot to "PlanHeader" component to permit more flexible display of title/className/description/etc
Adds "Enroll learners" option in the "GroupsPage/TableRow" component, emitting "enroll" when selected (handling not implemented yet)

References

Closes #12678

Reviewer guidance

Review the designs in Update views to create new "Groups" pages #12678 and look at the Groups tab; see that changes indicated there are in place.
Can rename and delete groups from new context menu on manage groups page (where you see users in the group)
See "Enroll learners" option in each group's options dropdown in the groups table, clicking it takes you to enroll learners in that row's group
Other tabs still work (Lessons, Quizzes)

…ners option

This is a string used in various places, which I noticed while needing to add it to address learningequality#12678 and not finding a common definition for the message.

This permits me to add the new description for the plan page and also gives makes it easy enough to include the link/button Groups has for "Learn more" next to the description text

nucleogenesis · 2024-10-02T23:11:03Z

kolibri/core/assets/src/mixins/commonCoreStrings.js

@@ -17,6 +17,11 @@ export const coreStrings = createTranslator('CommonCoreStrings', {
    context:
      'Button to cancel an action and return to the previous page. Usually this is the opposite of the save button which saves some piece of information.',
  },
+  learnMoreAction: {


Also defined in

MissingresourceAlert (kolibri-common)

DataPage (facility)

ProfilePage/index (user_profile)

Could update those in this PR, make a follow-up issue, or just revert this change and add this same string to the GroupsPage component

github-actions · 2024-10-02T23:35:52Z

Build Artifacts

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

pcenov · 2024-10-03T14:16:55Z

Hi @nucleogenesis,

I was able to identify the following issues, which I can also report as follow-ups:

When I am within a group and I select the Delete group option then I see some errors in the console and remain at the same page seeing "This group doesn't exist". For this scenario I would expect to be brought back to the Groups page while seeing the snackbar notification that the group is deleted:

delete.a.group.mp4

In Update views to create new "Groups" pages #12678 there's an image suggesting that while viewing a group one should be able to see the name of the class, while currently I can see the number of learners:

When I enroll all of the available learners in a group and then open again the "Enroll learners into..." page then I see the following text All users are already enrolled in this class. This is not caused by changes made here, but if possible we should change it to All users are already enrolled in this group

nucleogenesis · 2024-10-03T16:14:22Z

Thanks @pcenov your 2) item I will address here.

For 1) and 3) these are great feedback. I also found it odd that there was even a "this group doesn't exist" message there rather than sending us back to the groups page.

As for changing that string, I can update it quickly here unless @marcellamaki thinks this should be a part of some upcoming strings/i18n review

marcellamaki · 2024-10-03T16:43:53Z

@nucleogenesis for 3) are we currently using the "already enrolled in this class".... as an "error" string for class enrollments? might need to be an additional string, rather than a change?

…been removed see 805dc34 for last usage removed

This string is used only for referencing this page which is about enrolling learners in a particular group, not the class

nucleogenesis · 2024-10-03T17:47:19Z

@pcenov I went to fix (2) but ended up fixing all of them.

@marcellamaki that "All learners..." is only used on that component, so I've updated it from class to group accordingly.

Additionally, I've added a line that returns the user to the Groups page if they delete the group from that Group's member list page.

nucleogenesis · 2024-10-03T17:48:54Z

kolibri/plugins/coach/assets/src/views/plan/GroupMembersPage/index.vue

+            <KIcon
+              icon="classes"
+              class="class-name-icon"
+            />


I did not use a KLabeledIcon here because the classes icon is, by default, far too small compared to the text.

pcenov

Thanks @nucleogenesis - I confirm that the reported issues are fixed now - everything is implemented as specified in #12678

@radinamatic

marcellamaki

Thanks @nucleogenesis, looks great. I added one tiny non-blocking suggestion for comment phrasing, but fine to leave as-is if you think it's clearer

marcellamaki · 2024-10-04T14:11:25Z

kolibri/plugins/coach/assets/src/views/plan/GroupsPage/GroupRow.vue

+          default:
+            // eslint-disable-next-line no-console
+            console.warn(
+              `GroupRow: Tried to handleSelection of ${selectedOption}, but that isn't being handled.`,


that isn't being handled

totally a nit and not blocking, but, this means there's no matching case, right? I think if you were to say that, instead of "isn't being handled", it would be clearer

nucleogenesis added 9 commits October 2, 2024 13:51

add rename group and enroll learners to common coach strings

ae88eec

GroupsPage/GroupRow use updated strings for options; adds enroll lear…

1432fb5

…ners option

commonCoreStrings add learnMoreAction "Learn more"

ccebe04

This is a string used in various places, which I noticed while needing to add it to address learningequality#12678 and not finding a common definition for the message.

coach constants: add "ABOUT_GROUP" to GroupModals

2fa6fb2

PlanHeader: add a #header slot, allow override of title/description

d8d925c

This permits me to add the new description for the plan page and also gives makes it easy enough to include the link/button Groups has for "Learn more" next to the description text

GroupsPage: Adds "About groups" modal, $trs & handling

009cc9c

GroupsPage: incl class name w/ classes icon in PlanHeader#header slot

35e143f

GroupMembersPage: Add options menu for rename & delete group

b293b55

GroupsPage: handle enroll event from GroupRowTr

8efa165

nucleogenesis added the TODO: needs review Waiting for review label Oct 2, 2024

nucleogenesis added this to the Kolibri 0.18: Coach Resource Discovery milestone Oct 2, 2024

nucleogenesis requested review from marcellamaki and pcenov October 2, 2024 23:03

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

nucleogenesis commented Oct 2, 2024

View reviewed changes

nucleogenesis added the TAG: user strings Application text that gets translated label Oct 2, 2024

nucleogenesis added 4 commits October 3, 2024 10:40

GroupMembersPage: Swap classname icon text for # learners message

805dc34

commonCoachStrings: remove numberOfLearners as it's only usage has …

95ab3e5

…been removed see 805dc34 for last usage removed

GropuEnrollPage: "All users already enrolled in this ~class~ group"

3cbb569

This string is used only for referencing this page which is about enrolling learners in a particular group, not the class

GroupMembersPage: After group deleted, redirect to groups page

1fd6a5a

nucleogenesis commented Oct 3, 2024

View reviewed changes

pcenov approved these changes Oct 4, 2024

View reviewed changes

marcellamaki approved these changes Oct 4, 2024

View reviewed changes

nucleogenesis merged commit 9e8b8fb into learningequality:develop Oct 7, 2024
34 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

0.18: Groups view updates #12696

0.18: Groups view updates #12696

nucleogenesis commented Oct 2, 2024 •

edited

Loading

nucleogenesis Oct 2, 2024

github-actions bot commented Oct 2, 2024 •

edited

Loading

pcenov commented Oct 3, 2024

nucleogenesis commented Oct 3, 2024

marcellamaki commented Oct 3, 2024

nucleogenesis commented Oct 3, 2024

nucleogenesis Oct 3, 2024

pcenov left a comment

marcellamaki left a comment

marcellamaki Oct 4, 2024

0.18: Groups view updates #12696

0.18: Groups view updates #12696

Conversation

nucleogenesis commented Oct 2, 2024 • edited Loading

Summary

References

Reviewer guidance

nucleogenesis Oct 2, 2024

Choose a reason for hiding this comment

github-actions bot commented Oct 2, 2024 • edited Loading

Build Artifacts

pcenov commented Oct 3, 2024

nucleogenesis commented Oct 3, 2024

marcellamaki commented Oct 3, 2024

nucleogenesis commented Oct 3, 2024

nucleogenesis Oct 3, 2024

Choose a reason for hiding this comment

pcenov left a comment

Choose a reason for hiding this comment

marcellamaki left a comment

Choose a reason for hiding this comment

marcellamaki Oct 4, 2024

Choose a reason for hiding this comment

nucleogenesis commented Oct 2, 2024 •

edited

Loading

github-actions bot commented Oct 2, 2024 •

edited

Loading