Update folder selection logic to handle deep folders.

[rtibbles]

Summary

• Removes constant for max options per section, replaces with dynamic value calculated in the ResourceSelection component
• Moves all 'can select' logic into the ResourceSelection component
• Adds function to allow determination of select all indeterminate state
• Updates select all showing, based on whether selecting every displayed node could be added and still result in fewer than or equal to the total max number of question options.
• Update select all toggling to reuse existing toggleSelect function for consistency
• Update toggleSelect function to fetch additional data from the backend in case it is not annotated onto the topic (the case when the exercises are not direct children or in the first 12 children of the topic)
• Update showing the number of questions warning to be purely based on whether we're showing select all, if not, we show the message to explain why one or more items are not selectable
• Fix indeterminate state display for folders

Change to behaviour for clarity:

• Update to always show checkboxes for folder and resources
• Instead, disable checkboxes when the folder or resource cannot be selected
• Do not disable checkboxes if the item is currently selected
• Without this change, an indeterminately selected folder cannot be selected/deselected

Point for reflection: I tried to make it so that if a folder was indeterminately selected it and selecting it would exceed the max questions, it would deselect instead - this all worked, except that the KCheckbox did not properly update to the unselected state, so it made the UI more confusing. Instead, it retains its default behaviour, and instead displays the warning that too many questions are now selected.

Change to behaviour that can be reverted:

• I updated the max questions option per section to be dynamic based on how many questions you were trying to choose, so if you were trying to choose 5 questions to add to your quiz, you could only select a total of 50 questions. I can revert this, but it seemed a little bit sensible?

References

Fixes #12327

Reviewer guidance

Select, deselect, select all, unselect all, select some resources in a folder, and navigate to the parent. Just do all the permutations of selecting and deselecting and make sure weird edge cases don't arise.

In spite of the disabled checkboxes, it is still possible to get into a state where you have too many resources selected, but the number of paths to get there is much smaller.

---

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-.pex
Windows Installer (EXE)	kolibri-0.17.0a0.dev0+git.167.g9a42970a-windows-setup-unsigned.exe
Debian Package	kolibri_0.17.0a0.dev0+git.167.g9a42970a-0ubuntu1_all.deb
Mac Installer (DMG)	kolibri-0.17.0a0.dev0+git.167.g9a42970a-0.4.2.dmg
Android Package (APK)	kolibri-0.17.0a0.dev0+git.167.g9a42970a-0.1.3-debug.apk
TAR file	kolibri-0.17.0a0.dev0+git.167.g9a42970a.tar.gz
WHL file	kolibri-0.17.0a0.dev0+git.167.g9a42970a-py2.py3-none-any.whl

[nucleogenesis]

Code changes overall LGTM - just left some non-blocking questions/comments.

I'll give it a manual review before approving

[nucleogenesis]

Haven't tested yet - but just wondering what this might look like on a slow connection and if we might want to add a loading state here.

[nucleogenesis]

With that said, we probably need to work on adding loading states in various places...

2024-07-01.15-05-33.mp4

[nucleogenesis]

I think we can probably polish things in a follow-up - I left one typically blocking comment on a confusing UX.

Another thing that occurred to me is that a user can ultimately build up a large number of replacements. I'm curious to see how this performs on lower-end devices (separately from slower networks).

On my device, making a section with 50 questions and ~380 total replacements and then opening the replacements side panel, it takes AccordionContainer 3s to mount:

Perhaps we can also find a way to optimize/improve the AccordionContainer altogether -- perhaps the issue is actually the transition group?

[nucleogenesis]

This situation is weird:

• 12 questions "Number of questions"
• Select QA Channel > Exercises > CK12 Exercises (111 questions)
• Go back to QA Channel root, click the indeterminate checkbox on QA Channel
• Now I've been allowed to select 500+ questions, and I get an error and I have to click to deselect everything
• I shouldn't have been allowed to select that many questions with my current question count

I think maybe disabling the checkbox altogether might be better than this.

Or maybe we could instead just deselect the relevant selection when there are too many questions to add -- but this might be confusing in its own right (ie, "why does it select things in some cases but deselect in others?" might not have an obvious answer.)

[rtibbles]

Yes - I think I missed an edge case here and didn't ensure that checkboxes are always disabled for channels (even if we are displaying to show the descendant selector state).

[nucleogenesis]

With that said, we probably need to work on adding loading states in various places...

2024-07-01.15-05-33.mp4

[nucleogenesis]

Oh! I also forgot to mention there is a regression w/ the footer on the side panel -- I don't see how it'd be due to your PR as it is also seen on the replacements side panel - lemme know and I'll make a follow-up

[rtibbles]

@marcellamaki mentioned that she had fixed this in her PR #12374.

[marcellamaki]

yes, that will be fixed with the cards PR

[pcenov]

LGTM. In addition to what's already been commented above I noticed just the following which is probably not caused by the changes made here:

1. If I manually enter a number in the Number of questions field and then press the + button I am seeing a 1 added to the right of the number, instead of actually increasing it by 1:

2024-07-03_15-37-21.mp4

Also perhaps at some point the default text 'You can only select a total of 100 questions or fewer.' should be further rephrased as the distinction between the number of selected questions and the number which can actually be added is not crystal clear.

[marcellamaki]

I've added a (referenced) follow up issue for @pcenov's comment above . @nucleogenesis can you go through the code one more time and make sure all of your concerns are addressed?

[nucleogenesis]

The code LGTM here - my concern around the checkbox state cycle was discussed and we'll see later on if we feel the need to dig into working out how to make indeterminate go to unchecked in certain cases.