Tables loading

[MisRob]

Summary

Adds a circular loader to tables in

• Coach > Plan > Lessons
• Coach > Plan > Groups

by

• utilizing composables that store local loading state shared only between relevant components

Includes related refactoring of handlers into composable functions to keep all related logic in one place + minor cleanup.

Comments

@nucleogenesis Note that this doesn't add the circular loader to tables to "Facility > Classes > Class > Enroll learners" and "Facility > Classes > Class > Assign coaches" as requested in #10785, since we don't only load tables but also other parts of the page so in these two particular cases, UX would be inconsistent with other places in Kolibri and perhaps confusing if I used just table loaders (see "Enroll learners into ''" and missing page title in the top bar):

In comparison, this is our current version with the linear loader:

I honestly don't know which version is better, neither looks ideal. From this and some recent discussions I noticed, I think we could improve page loaders in general however it'd be good to have input from the design team at first before refactoring so that we plan for something consistent in the whole app, and also in some cases like here, it might require changes to what data we load at what times.

Meanwhile, I'd suggest following the recommendation to use linear loaders https://design-system.learningequality.org/loaders/#linearloaders for the whole page load rather than spending time on something that we don't seem to be clear about yet, if there are no strong objections. Does that make sense?

References

Closes #10785

Reviewer guidance

Test all states of tables (with data, no data, filters) in

• Coach > Plan > Lessons
• Coach > Plan > Groups

To see the table loader, you will likely need to throttle your network.

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]

Build Artifacts

Asset type	Download link
PEX file	kolibri-0.16.0a18.dev0_git.60.g3ff95c54.pex
Windows Installer (EXE)	kolibri-0.16.0a18.dev0+git.60.g3ff95c54-unsigned.exe
Debian Package	kolibri_0.16.0a18.dev0+git.60.g3ff95c54-0ubuntu1_all.deb
Mac Installer (DMG)	kolibri-0.16.0a18.dev0+git.60.g3ff95c54-0.3.0.dmg
TAR file	kolibri-0.16.0a18.dev0+git.60.g3ff95c54.tar.gz
WHL file	kolibri-0.16.0a18.dev0+git.60.g3ff95c54-py2.py3-none-any.whl

[nucleogenesis]

Manual testing and code both look great! Cheers for the first of hopefully many shifts in Coach from Vuex to the composition API <3

[nucleogenesis]

It is a little strange that there are groups showing while the loader spins. Ideally, the table loading state would hide existing data (or at least "disable" it or otherwise indicate that it is currently part of the loading dataset)

Maybe a v-if="!groupsAreLoading" on the tbody here would do it?

[nucleogenesis]

In testing I'm not actually seeing the group while the loader is shown 🤷🏻

[MisRob]

I don't understand, could you please elaborate a bit more?

[MisRob]

Maybe a v-if="!groupsAreLoading" on the tbody here would do it?

In any case, I think this won't harm anything so no problem to commit it

[MisRob]

Perhaps it'd make more sense to have this implemented rather in CoreTable, though? Not only to have a condition to show the loader when dataLoading is truthy but also to not show the table body in this case? I think that may prevent some glitches (maybe in regards to cached data?) by ensuring that table data is never displayed as long as dataLoading is truthy.

[nucleogenesis]

In your screencast, when you go to the Group tab, "Group 1" is visible with the loader underneath it. I think the loader being there should hide the content on the table until it is done loading.

However, I was not able to replicate it and tried as best I could so I don't think it's a blocking issue at all.

Perhaps it'd make more sense to have this implemented rather in CoreTable, though?

Yeah I think you're right there for sure. That might be a good follow-up issue as it'd require regression testing across a bunch of tables and this is good to go otherwise. What do you think?

[MisRob]

In your screencast, when you go to the Group tab, "Group 1" is visible with the loader underneath it. I think the loader being there should hide the content on the table until it is done loading.

However, I was not able to replicate it and tried as best I could so I don't think it's a blocking issue at all.

Ah yes, I see now. Yes, that's right. I think it might be related to the way I prepared the recording when I added a piece of code to delay loading - it's possible that I placed it in the wrong place as I couldn't reproduce it either right now. Or it could be related to some kind of cache - we can resolve that in a follow-up and do regression testing, yes. I will open the issue.

[MisRob]

#10993