Skip to content

Commit

Permalink
Add Tags to Catalog Criteria (#10007)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@thsparks
thsparks committed May 10, 2024
1 parent 3711bcb commit 1b14c18
Show file tree
Hide file tree
Showing 12 changed files with 212 additions and 54 deletions.
5 changes: 5 additions & 0 deletions common-docs/teachertool/catalog-shared.json
View file
Expand Up @@ -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",
Expand All @@ -27,6 +28,7 @@
"description": "The project contains at least the specified number of comments.",
"docPath": "/teachertool",
"maxCount": 1,
"tags": ["General"],
"params": [
{
"name": "count",
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand Down
1 change: 1 addition & 0 deletions common-docs/teachertool/test/catalog-shared.json
View file
Expand Up @@ -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",
Expand Down
23 changes: 16 additions & 7 deletions react-common/components/controls/Accordion/Accordion.tsx
View file
@@ -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 {
Expand All @@ -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}
Expand All @@ -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 (
Expand Down Expand Up @@ -150,4 +159,4 @@ export const AccordionPanel = (props: AccordionPanelProps) => {
{children}
</div>
);
}
}
39 changes: 23 additions & 16 deletions react-common/components/controls/Accordion/context.tsx
View file
@@ -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}>
Expand All @@ -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 => (
{
Expand All @@ -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() {
Expand All @@ -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),
};
}
}
}
122 changes: 95 additions & 27 deletions teachertool/src/components/CatalogOverlay.tsx
View file
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 => {
Expand Down Expand Up @@ -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>
);
};

Expand Down

0 comments on commit 1b14c18

Please sign in to comment.