learningequality / kolibri

Kolibri Learning Platform: the offline app for universal education
https://learningequality.org/kolibri/
MIT License
815 stars 694 forks source link

0.18: Groups view updates #12696

Closed nucleogenesis closed 1 month ago

nucleogenesis commented 1 month ago

Summary

https://github.com/user-attachments/assets/da845e9c-6a11-4dfc-8166-a5495f82aa18

References

Closes #12678

Reviewer guidance

github-actions[bot] commented 1 month ago

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 commented 1 month ago

Hi @nucleogenesis,

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

  1. 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:

https://github.com/user-attachments/assets/19982907-26d1-4670-8d89-ff97e6466c11

  1. In https://github.com/learningequality/kolibri/issues/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:

missing class name

  1. 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

enrolled in group

nucleogenesis commented 1 month ago

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 commented 1 month ago

@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?

nucleogenesis commented 1 month ago

@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.