Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Improve loading UX on some places in Coach #11201

Merged
merged 6 commits into from
Sep 27, 2023

Conversation

MisRob
Copy link
Member

@MisRob MisRob commented Sep 6, 2023

Summary

  • Updates Kolibri to the KDS version Add supporting code for Kolibri loaders kolibri-design-system#448 (temporary fork reference, will be replaced) to include work done in support for this PR
  • Fixes and improvements of toggling visibility in the lessons list in Coach > Plan:
    • NaN B in size column doesn't show anymore
      • By re-ordering async events
    • Addresses unresponsive toggle switch on slow networks and prevents from repeated clicks
      • By showing a loader and re-ordering async events to achieve consistency

Last item affects UX on faster networks as well. To prevent from jarring UX, a loader will be displayed for at least 2 seconds even when all requests have finished. Hopefully this helps overall to make UX feel trustworthy taking into account that toggling a lesson visibility is an important operation that would ideally give coaches some feedback.

Before After
Slow network slow-network-before slow-network-after
Fast network fast-network-before fast-network-after

References

Fixes #10797

Reviewer guidance

Toggle lessons visibility in Plan > Coach on fast network as well as slow network (you can use network throttling in browser dev tools)


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

  • Automated test coverage is satisfactory
  • 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 github-actions bot added APP: Learn Re: Learn App (content, quizzes, lessons, etc.) APP: Coach Re: Coach App (lessons, quizzes, groups, reports, etc.) SIZE: medium labels Sep 6, 2023
@MisRob MisRob changed the title Coach loading Improve loading UX on some places in Coach Sep 6, 2023
@MisRob MisRob changed the title Improve loading UX on some places in Coach [DO NOT MERGE] Improve loading UX on some places in Coach Sep 6, 2023
- Resolves seemingly unresponsive toggle switch
on slow networks and prevents from repeated clicks.
- Includes related re-ordering of async events
to achieve consistent UX.
@MisRob MisRob added the TODO: needs review Waiting for review label Sep 7, 2023
@MisRob MisRob marked this pull request as ready for review September 7, 2023 13:29
@MisRob MisRob requested a review from pcenov September 7, 2023 13:42
Copy link
Member

@pcenov pcenov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @MisRob I confirm that the issue described in #10797 is now fixed!

@MisRob MisRob assigned MisRob and unassigned MisRob Sep 12, 2023
Copy link
Member

@nucleogenesis nucleogenesis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great. I really love seeing the show being used in a real use case. It looks like a real pleasure to use and I hope I get a chance to integrate it into other places where our loaders are used!

@MisRob MisRob changed the title [DO NOT MERGE] Improve loading UX on some places in Coach Improve loading UX on some places in Coach Sep 27, 2023
@MisRob
Copy link
Member Author

MisRob commented Sep 27, 2023

KDS reference updated

@MisRob MisRob merged commit 6257a5c into learningequality:release-v0.16.x Sep 27, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
APP: Coach Re: Coach App (lessons, quizzes, groups, reports, etc.) APP: Learn Re: Learn App (content, quizzes, lessons, etc.) SIZE: medium TODO: needs review Waiting for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"NaN B" displayed in coach when toggling lesson visibility
3 participants