Quiz creation bookmark selection

[ozer550]

Summary

• Explored various other path ways to implement the Bookmarks support with sidePanel for quiz selection.
• Added Query parameter in the route to support conditional rendering of Bookmarks.
• Render Bookmarks card and exercises Bookmarked according to the boolean value of the query parameter.

References

closes #11818

Reviewer guidance

• Bookmark some of the exercises from the existing channels.
• Go to Quiz Section.
• When you open the sidepanel to add questions, you will also see an option to add bookmarked exercises.

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.0b13.dev0_git.500.g291660b7.pex
Windows Installer (EXE)	kolibri-0.16.0b13.dev0+git.500.g291660b7-unsigned.exe
Debian Package	kolibri_0.16.0b13.dev0+git.500.g291660b7-0ubuntu1_all.deb
Mac Installer (DMG)	kolibri-0.16.0b13.dev0+git.500.g291660b7-0.3.0.dmg
Android Package (APK)	kolibri-0.16.0b13.dev0+git.500.g291660b7-0.1.0-debug.apk
TAR file	kolibri-0.16.0b13.dev0+git.500.g291660b7.tar.gz
WHL file	kolibri-0.16.0b13.dev0+git.500.g291660b7-py2.py3-none-any.whl

[nucleogenesis]

Some follow-up notes from a cohack today where we discussed another approach -- overall, I think that the duplication is maybe a bit too much and that we can find another way to approach this more simply.

I'll describe two possible paths here

1 - The path we explored together - bookmarks query parameter

• Update the getBookmarksLink property to return the current route, but place a query parameter on it called something like showBookmarks
• Within the contentList computed property, conditionally return the list of bookmarks if that query parameter is in place
• Update the conditions for showing the "Bookmarks" button-card so that it doesn't show while listing the bookmarks -- could be a computed property showBookmarksButton that uses that same logic, but also takes into consideration whether or not we're listing the bookmarks (which could be its own computed property as well showBookmarks)
• Note that these computed properties can also be leveraged to update the text on the screen, for example on this Figma where it says "Channel name" we may instead see "Bookmarks" as the title for the list.

Pros

• Less duplication
• Don't need to use a new route for the side panel
• Faster to get things working

Cons

• Adds more bloat to the sizable ResourceSelection component
• Comments explaining complexity may not always be helpful
• Just putting a query parameter on the route to change the page's behavior is a bit brittle -- if we ever allow bookmarking whole topics, this may not be so easy to update
• This might get a bit more hairy when we implement search (as we may find that will have another query parameter searchTerm or something that we'd have to conditionalize against for what we're showing, how and where.
• If & when we need to tweak designs you can find yourself wrestling with a series of boolean computed properties and managing a bunch of conditionals throughout the template

2 - A path more similar to the one you have here

• Rather than copying and pasting the bulk of the ResourceSelection component here (including the footer, and such), you could trim the new ShowBookMarkedResources component down to just basically be the title text (eg "Bookmarks") and the ContentCardList components, then pass the values needed in as props from ResourceSelection. This would result also in needing to leave ResourceSelection as the component for the bookmarks route
• Then in ResourceSelection you could conditionally display that new component in place of the similar section of template that it has (ie, the title, breadcrumbs, ContentCardList, etc)
• You could even consider making three total child components, one each for the three possible ways in which ResourceSelection might display all the things.

In this instance, the pattern would result in needing to pass in some of the methods that we're currently just using directly within ResourceSelection for the interaction handling rather than importing them all in each of the child components.

As a rough example:

<template>
    <div ...>
        <div v-if="loading">
            <KCircularLoader />
        </div>
        <div v-else>
            <component :is="visibleComponent" :isChecked="isChecked" :selectAllChecked... />
        </div>
    <!-- the footer business -->
</template>

<script>
/* imports and such */
setup {
    // The component which is shown to list the items
    const visibleComponent = computed(() => {
        if(!topicId) {
            if(route.value.name == PageNames.BOOK_MARKED_RESOURCES) {
                   // The component that fetches & lists bookmarks
                    return ShowBookMarkedResources;
            } else {
                   // The component that shows the Bookmarks link card 
                   // and fetches & shows channels
                    return ShowChannels;
            }
        } else {
            // The component that fetches and shows the children for the current
            // topicId value & the breadcrumbs
            return ShowTopic;
        }
    });
/* everything else */

Pros

• The logic for each individual way in which we might show contents listed out is encapsulated in its own component
• The ResourceSelection component has its own responsibilities reduced as it will no longer handle the fetching but, rather, will delegate the heavy lifting to those child components
• It can be easier to reason about what is happening and debug using dev tools when logic is spread around like this
• This can be more extensible -- that is to say that it would then be relatively easy to add a similar "search" route and a new ShowQuizSearchResults component, which may indeed have its own specific behavioral needs.

Cons

• Will require more upfront work to dismantle the current ResourceSelection component so that the fetching business happens in the child components
• As you begin to pull at the seams of the existing component, you'll have to make the decision around which props can be (or need to be) defined in which component for their own purposes, such as the passing down of various things imported from useQuizResources

---

Overall, I think that the second path is probably the "golden path" in that it abides common patterns and principles in Vue development such as encapsulation & the separation of concerns.

However, the first path is probably much faster and simpler to implement -- but only for now as we may find that we wish we'd taken the second path a few months down the line when we have to implement the Coach metadata search business.

[nucleogenesis]

Just a couple of stylistic comments, one blocking, another not. Also one request for adding comments.

Overall, looking at this diff the solution doesn't feel like it adds too much complexity as I'd worried it might.

Rather, I think that there is just some complexity inherent to this component that we'd do well to clean up sooner than later and this particular approach isn't the source of the complexity.

[nucleogenesis]

I'd also love to see a comment here explaining what this is used for to give some context around how this whole solution works (ie, we're using the query parameter as a " route " of sorts to determine if we should be showing bookmarks or not.

Overall I think that @AlexVelezLl might do well to take inspiration from this approach for the work on #11786

[nucleogenesis]

LGTM after the updates - thanks @ozer550