Add Tags to Catalog Criteria (#10007)

This change adds tags to criteria in the catalog. The schema is setup to support criteria having multiple tags, but for now we only adjust our display based on the first tag. This may change if we add more criteria and start using more granular tags, but it's not necessary given the current catalog.

If a criterion doesn't have any tags, it gets assigned an "Other" tag. Currently, all visible catalog criteria have tags, so the other tag is never shown.

Initially, the first tag in the catalog is expanded, but the tool remembers which tags the user expands/collapses and preserves those for the next time the catalog is opened.

common-docs/teachertool/catalog-shared.json

@@ -6,6 +6,7 @@
             "template": "${Block} used ${count} times",
             "description": "This block was used the specified number of times in your project.",
             "docPath": "/teachertool",
+            "tags": ["General"],
             "params": [
                 {
                     "name": "block",
@@ -27,6 +28,7 @@
             "description": "The project contains at least the specified number of comments.",
             "docPath": "/teachertool",
             "maxCount": 1,
+            "tags": ["General"],
             "params": [
                 {
                     "name": "count",
@@ -43,6 +45,7 @@
             "docPath": "/teachertool",
             "description": "The program uses at least this many loops of any kind (for, repeat, while, or for-of).",
             "maxCount": 1,
+            "tags": ["Code Elements"],
             "params": [
                 {
                     "name": "count",
@@ -59,6 +62,7 @@
             "docPath": "/teachertool",
             "description": "At least this many user-defined functions are created and called.",
             "maxCount": 1,
+            "tags": ["Code Elements"],
             "params": [
                 {
                     "name": "count",
@@ -75,6 +79,7 @@
             "docPath": "/teachertool",
             "description": "The program creates and uses at least this many user-defined variables.",
             "maxCount": 1,
+            "tags": ["Code Elements"],
             "params": [
                 {
                     "name": "count",

common-docs/teachertool/test/catalog-shared.json

@@ -7,6 +7,7 @@
             "description": "Experimental: AI outputs may not be accurate. Use with caution and always review responses.",
             "docPath": "/teachertool",
             "maxCount": 10,
+            "tags": ["General"],
             "params": [
                 {
                     "name": "question",

react-common/components/controls/Accordion/Accordion.tsx

@@ -1,15 +1,19 @@
 import * as React from "react";
 import { ContainerProps, classList, fireClickOnEnter } from "../../util";
 import { useId } from "../../../hooks/useId";
-import { AccordionProvider, clearExpanded, setExpanded, useAccordionDispatch, useAccordionState } from "./context";
+import { AccordionProvider, removeExpanded, setExpanded, useAccordionDispatch, useAccordionState } from "./context";
 
 export interface AccordionProps extends ContainerProps {
+    multiExpand?: boolean;
+    defaultExpandedIds?: string[];
     children?: React.ReactElement<AccordionItemProps>[] | React.ReactElement<AccordionItemProps>;
 }
 
 export interface AccordionItemProps extends ContainerProps {
     children?: [React.ReactElement<AccordionHeaderProps>, React.ReactElement<AccordionPanelProps>];
     noChevron?: boolean;
+    itemId?: string;
+    onExpandToggled?: (expanded: boolean) => void;
 }
 
 export interface AccordionHeaderProps extends ContainerProps {
@@ -27,10 +31,12 @@ export const Accordion = (props: AccordionProps) => {
         ariaHidden,
         ariaDescribedBy,
         role,
+        multiExpand,
+        defaultExpandedIds
     } = props;
 
     return (
-        <AccordionProvider>
+        <AccordionProvider multiExpand={multiExpand} defaultExpandedIds={defaultExpandedIds}>
             <div
                 className={classList("common-accordion", className)}
                 id={id}
@@ -54,23 +60,26 @@ export const AccordionItem = (props: AccordionItemProps) => {
         ariaHidden,
         ariaDescribedBy,
         role,
-        noChevron
+        noChevron,
+        itemId,
+        onExpandToggled,
     } = props;
 
     const { expanded } = useAccordionState();
     const dispatch = useAccordionDispatch();
 
-    const panelId = useId();
+    const panelId = itemId ?? useId();
     const mappedChildren = React.Children.toArray(children);
-    const isExpanded = expanded === panelId;
+    const isExpanded = expanded.indexOf(panelId) !== -1;
 
     const onHeaderClick = React.useCallback(() => {
         if (isExpanded) {
-            dispatch(clearExpanded());
+            dispatch(removeExpanded(panelId));
         }
         else {
             dispatch(setExpanded(panelId));
         }
+        onExpandToggled?.(!isExpanded);
     }, [isExpanded]);
 
     return (
@@ -150,4 +159,4 @@ export const AccordionPanel = (props: AccordionPanelProps) => {
             {children}
         </div>
     );
-}
+}

react-common/components/controls/Accordion/context.tsx

@@ -1,17 +1,22 @@
 import * as React from "react";
 
 interface AccordionState {
-    expanded?: string;
+    multiExpand?: boolean;
+    expanded: string[];
 }
 
 const AccordionStateContext = React.createContext<AccordionState>(null);
 const AccordionDispatchContext = React.createContext<(action: Action) => void>(null);
 
-export const AccordionProvider = ({ children }: React.PropsWithChildren<{}>) => {
-    const [state, dispatch] = React.useReducer(
-        accordionReducer,
-        {}
-    );
+export const AccordionProvider = ({
+    multiExpand,
+    defaultExpandedIds,
+    children,
+}: React.PropsWithChildren<{ multiExpand?: boolean; defaultExpandedIds?: string[] }>) => {
+    const [state, dispatch] = React.useReducer(accordionReducer, {
+        expanded: defaultExpandedIds ?? [],
+        multiExpand,
+    });
 
     return (
         <AccordionStateContext.Provider value={state}>
@@ -27,11 +32,12 @@ type SetExpanded = {
     id: string;
 };
 
-type ClearExpanded = {
-    type: "CLEAR_EXPANDED";
+type RemoveExpanded = {
+    type: "REMOVE_EXPANDED";
+    id: string;
 };
 
-type Action = SetExpanded | ClearExpanded;
+type Action = SetExpanded | RemoveExpanded;
 
 export const setExpanded = (id: string): SetExpanded => (
     {
@@ -40,14 +46,15 @@ export const setExpanded = (id: string): SetExpanded => (
     }
 );
 
-export const clearExpanded = (): ClearExpanded => (
+export const removeExpanded = (id: string): RemoveExpanded => (
     {
-        type: "CLEAR_EXPANDED"
+        type: "REMOVE_EXPANDED",
+        id
     }
 );
 
 export function useAccordionState() {
-    return React.useContext(AccordionStateContext)
+    return React.useContext(AccordionStateContext);
 }
 
 export function useAccordionDispatch() {
@@ -59,12 +66,12 @@ function accordionReducer(state: AccordionState, action: Action): AccordionState
         case "SET_EXPANDED":
             return {
                 ...state,
-                expanded: action.id
+                expanded: state.multiExpand ? [...state.expanded, action.id] : [action.id],
             };
-        case "CLEAR_EXPANDED":
+        case "REMOVE_EXPANDED":
             return {
                 ...state,
-                expanded: undefined
+                expanded: state.expanded.filter((id) => id !== action.id),
             };
     }
-}
+}

teachertool/src/components/CatalogOverlay.tsx

@@ -6,11 +6,15 @@ import { getCatalogCriteria } from "../state/helpers";
 import { ReadOnlyCriteriaDisplay } from "./ReadonlyCriteriaDisplay";
 import { Strings } from "../constants";
 import { Button } from "react-common/components/controls/Button";
-import { getReadableCriteriaTemplate, makeToast } from "../utils";
+import { Accordion } from "react-common/components/controls/Accordion";
+import { getReadableCriteriaTemplate } from "../utils";
 import { setCatalogOpen } from "../transforms/setCatalogOpen";
 import { classList } from "react-common/components/util";
 import { announceToScreenReader } from "../transforms/announceToScreenReader";
 import { FocusTrap } from "react-common/components/controls/FocusTrap";
+import { logError } from "../services/loggingService";
+import { ErrorCode } from "../types/errorCode";
+import { addExpandedCatalogTag, getExpandedCatalogTags, removeExpandedCatalogTag } from "../services/storageService";
 import css from "./styling/CatalogOverlay.module.scss";
 
 interface CatalogHeaderProps {
@@ -67,16 +71,59 @@ const CatalogItemLabel: React.FC<CatalogItemLabelProps> = ({ catalogCriteria, is
     );
 };
 
+interface CatalogItemProps {
+    catalogCriteria: CatalogCriteria;
+    recentlyAddedIds: pxsim.Map<NodeJS.Timeout>;
+    onItemClicked: (c: CatalogCriteria) => void;
+}
+const CatalogItem: React.FC<CatalogItemProps> = ({ catalogCriteria, recentlyAddedIds, onItemClicked }) => {
+    const { state: teacherTool } = useContext(AppStateContext);
+
+    const existingInstanceCount = teacherTool.checklist.criteria.filter(
+        i => i.catalogCriteriaId === catalogCriteria.id
+    ).length;
+    const isMaxed = catalogCriteria.maxCount !== undefined && existingInstanceCount >= catalogCriteria.maxCount;
+    return catalogCriteria.template ? (
+        <Button
+            title={getReadableCriteriaTemplate(catalogCriteria)}
+            key={catalogCriteria.id}
+            className={css["catalog-item"]}
+            label={
+                <CatalogItemLabel
+                    catalogCriteria={catalogCriteria}
+                    isMaxed={isMaxed}
+                    recentlyAdded={recentlyAddedIds[catalogCriteria.id] !== undefined}
+                />
+            }
+            onClick={() => onItemClicked(catalogCriteria)}
+            disabled={isMaxed}
+        />
+    ) : null;
+};
+
 const CatalogList: React.FC = () => {
     const { state: teacherTool } = useContext(AppStateContext);
 
     const recentlyAddedWindowMs = 500;
     const [recentlyAddedIds, setRecentlyAddedIds] = useState<pxsim.Map<NodeJS.Timeout>>({});
 
-    const criteria = useMemo<CatalogCriteria[]>(
-        () => getCatalogCriteria(teacherTool),
-        [teacherTool.catalog, teacherTool.checklist]
-    );
+    // For now, we only look at the first tag of each criteria.
+    const criteriaGroupedByTag = useMemo<pxt.Map<CatalogCriteria[]>>(() => {
+        const grouped: pxt.Map<CatalogCriteria[]> = {};
+        getCatalogCriteria(teacherTool)?.forEach(c => {
+            if (!c.tags || c.tags.length === 0) {
+                logError(ErrorCode.missingTag, { message: "Catalog criteria missing tag", criteria: c });
+                return;
+            }
+
+            const tag = c.tags[0];
+            if (!grouped[tag]) {
+                grouped[tag] = [];
+            }
+            grouped[tag].push(c);
+        });
+        return grouped;
+    }, [teacherTool.catalog]);
 
     function updateRecentlyAddedValue(id: string, value: NodeJS.Timeout | undefined) {
         setRecentlyAddedIds(prevState => {
@@ -106,34 +153,55 @@ const CatalogList: React.FC = () => {
         announceToScreenReader(lf("Added '{0}' to checklist.", getReadableCriteriaTemplate(c)));
     }
 
+    function getItemIdForTag(tag: string) {
+        return `accordion-item-${tag}`;
+    }
+
+    function onTagExpandToggled(tag: string, expanded: boolean) {
+        if (expanded) {
+            addExpandedCatalogTag(tag);
+        } else {
+            removeExpandedCatalogTag(tag);
+        }
+    }
+
+    const tags = Object.keys(criteriaGroupedByTag);
+    if (tags.length === 0) {
+        logError(ErrorCode.noCatalogCriteria);
+        return null;
+    }
+
+    let expandedTags = getExpandedCatalogTags();
+    if (!expandedTags) {
+        // If we haven't saved an expanded set, default expand the first one.
+        addExpandedCatalogTag(tags[0]);
+        expandedTags = [tags[0]];
+    }
+
+    const expandedIds = expandedTags.map(t => getItemIdForTag(t));
     return (
-        <div className={css["catalog-list"]}>
-            {criteria.map(c => {
-                const existingInstanceCount = teacherTool.checklist.criteria.filter(
-                    i => i.catalogCriteriaId === c.id
-                ).length;
-                const isMaxed = c.maxCount !== undefined && existingInstanceCount >= c.maxCount;
+        <Accordion className={css["catalog-list"]} multiExpand={true} defaultExpandedIds={expandedIds}>
+            {tags.map(tag => {
                 return (
-                    c.template && (
-                        <Button
-                            id={`criteria_${c.id}`}
-                            title={getReadableCriteriaTemplate(c)}
-                            key={c.id}
-                            className={css["catalog-item"]}
-                            label={
-                                <CatalogItemLabel
+                    <Accordion.Item
+                        itemId={getItemIdForTag(tag)}
+                        onExpandToggled={expanded => onTagExpandToggled(tag, expanded)}
+                        key={getItemIdForTag(tag)}
+                    >
+                        <Accordion.Header>{tag}</Accordion.Header>
+                        <Accordion.Panel>
+                            {criteriaGroupedByTag[tag].map(c => (
+                                <CatalogItem
                                     catalogCriteria={c}
-                                    isMaxed={isMaxed}
-                                    recentlyAdded={recentlyAddedIds[c.id] !== undefined}
+                                    recentlyAddedIds={recentlyAddedIds}
+                                    onItemClicked={onItemClicked}
                                 />
-                            }
-                            onClick={() => onItemClicked(c)}
-                            disabled={isMaxed}
-                        />
-                    )
+                            ))}
+                        </Accordion.Panel>
+                    </Accordion.Item>
                 );
             })}
-        </div>
+        </Accordion>
     );
 };