learningequality / kolibri

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

Coach main navigation refactor #12775

Closed AlexVelezLl closed 1 week ago

AlexVelezLl commented 3 weeks ago

Summary

https://github.com/user-attachments/assets/805d43c7-82a4-4d78-9023-38d4a2a384b6

References

Closes #12729 and #12801

Reviewer guidance

For code review:

For QA:

Follow ups

Testing checklist

PR process

Reviewer checklist

github-actions[bot] commented 3 weeks ago

Build Artifacts

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

Hi @AlexVelezLl, I was able to identify the following issues, which I can also report separately if not in scope for this PR:

  1. The tabs in the navigation should go to the second line when there isn't enough space, and only after that we should be showing the ... button. See the video:

https://github.com/user-attachments/assets/d324f647-e98b-4ed3-9397-85248fec9568

  1. The 'Edit lesson' option actually creates a brand new lesson:

https://github.com/user-attachments/assets/28db4ea9-1ed6-49de-adb7-1b9b8a9ba2ac

  1. Nothing happens when I click the 'Preview' button for a quiz:

https://github.com/user-attachments/assets/cc90784b-7c80-4dc6-8245-1c3ad5ab7e75

  1. When I delete a quiz I am not immediately brought back to the Quizzes page:

https://github.com/user-attachments/assets/bd10fe47-99c1-4d54-84e0-5b6f24f6c01b

  1. When I make a copy of a quiz and I click the 'All quizzes' link then I see an empty page:

https://github.com/user-attachments/assets/1ed3ff38-bcc0-4e49-bad4-03be1d7ec6b5

  1. When there is only one class in the facility and I navigate from another plugin to a page in the Coach plugin, then I always see the Classes page before going to the desired page which should be avoided if possible. This is especially obvious in slow 3g network throttling:

https://github.com/user-attachments/assets/a45c163f-1e55-4382-b266-7f9f30879c09

AlexVelezLl commented 1 week ago

Thank you @pcenov!

  1. Yes, I think we can add this as a follow up issue as this is an existing behaviour even in the current implementation.
  2. I will fix this!
  3. This is an issue from #12685. ~I think we could just hide this button for now~. I just linked the preview page!
  4. Will fix this :)
  5. ~Will fix this too~. This is also happening in develop so is a general bug when we copy a quiz and go back to the quizzes pages rather than a bug introduced in this PR. We can open another issue for this.
  6. ~Is also possible to replicate this in the current version in develop? Or is something that has been introduced in this PR~. This is also happening in develop.
AlexVelezLl commented 1 week ago

I have fixed 2, 3 and 4 @pcenov :). 1, 5 and 6 are issues that also happen in develop so I think it will be better to have different issues for them.

pcenov commented 1 week ago

Thanks @AlexVelezLl - I confirm that 2, 3 and 4 are fixed now and I have filed follow-up issues for the rest.

While regression testing today I noticed the following issues, which again if not caused by changes made here, I'll report separately:

  1. The Export as CSV button in Coach > Lessons is not working, seeing the following error in the console: TypeError: Cannot read properties of undefined (reading 'map'):

https://github.com/user-attachments/assets/0a1f497c-d47d-4650-9e6f-2c4f83ce3616

  1. When creating/editing a quiz the icons are positioned right next to the 'Select all':

icons

  1. Attempting to view the Learners page as a Learner user results in seeing a blank page, instead of seeing the message "You must be signed in as an admin or coach to view this page":

learners

AlexVelezLl commented 1 week ago

Thank you @pcenov! I have fixed numbers 1 and 2. Number 1 was also happening in develop but just fixed this as this was a quick fix.

Number 3 is part of a broader issue which is that in coach we dont have a not found or permissions error page in general, not as part of this PR, so I think this could be reported separately.

pcenov commented 1 week ago

Thanks @AlexVelezLl - I've verified that points 1 and 2 are fixed now and I didn't see any other issues while regression testing. I've reported separately the permissions issue.