diff --git a/src/articles/ArticlePreview.js b/src/articles/ArticlePreview.js index 271c4ca5c..89921b653 100644 --- a/src/articles/ArticlePreview.js +++ b/src/articles/ArticlePreview.js @@ -6,6 +6,7 @@ import "react-toastify/dist/ReactToastify.css"; import { isMobile } from "../utils/misc/browserDetection"; import * as s from "./ArticlePreview.sc"; import { MetaStrip, MetaItem, MetaLink, MetaTag } from "../components/MetaStrip.sc"; +import { classColor } from "../utils/misc/classColor"; import RedirectionNotificationModal from "../components/redirect_notification/RedirectionNotificationModal"; import Feature from "../features/Feature"; import strings from "../i18n/definitions"; @@ -295,9 +296,14 @@ export default function ArticlePreview({ // Which class a text came from. Only for a student in more than one class: // with a single class it is the same answer on every row, and the tab itself // already says it. Computed here because both layouts below render a strip. + // The class's own colour, matching its chip in the filter bar above -- the + // pair is what lets you skim the list for one class instead of reading every + // label. Colour supplements the name, never replaces it. const classTags = showClassNames ? (article.from_classes || []).map((each) => ( - {each.name} + + {each.name} + )) : null; diff --git a/src/articles/ClassroomArticles.js b/src/articles/ClassroomArticles.js index 36b91e393..6713ba345 100644 --- a/src/articles/ClassroomArticles.js +++ b/src/articles/ClassroomArticles.js @@ -13,11 +13,15 @@ import { APIContext } from "../contexts/APIContext"; import ClassroomOtherLanguages, { otherLanguageOptions, } from "./ClassroomOtherLanguages"; +import { ALL, buildClassFilters, filterTexts } from "../utils/misc/classFilters"; +import { classColorTint } from "../utils/misc/classColor"; +import * as f from "../teacher/styledComponents/TextFilterBar.sc"; export default function ClassroomArticles() { const api = useContext(APIContext); const [articleList, setArticleList] = useState(null); const [student, setStudent] = useState(null); + const [activeClass, setActiveClass] = useState(ALL); let originalList = articleList; @@ -93,12 +97,40 @@ export default function ClassroomArticles() { const inMoreThanOneClass = student.cohorts.length > 1; + // With several classes the list is a merge, and a term's reading from each + // adds up: filter by class, the same chips the teacher gets on My Texts. + const classFilters = inMoreThanOneClass + ? buildClassFilters(articleList, { allLabel: "All classes" }) + : []; + const visible = inMoreThanOneClass ? filterTexts(articleList, activeClass) : articleList; + return ( <>

+ {inMoreThanOneClass && ( + + {classFilters.map((filter) => { + const tint = filter.id === ALL ? {} : classColorTint(filter.id); + return ( + setActiveClass(filter.id)} + > + {filter.name} + {filter.count} + + ); + })} + + )} - {articleList.map((each) => ( + {visible.map((each) => ( {filter.count} ))} - +
+ +
)} diff --git a/src/teacher/styledComponents/TextFilterBar.sc.js b/src/teacher/styledComponents/TextFilterBar.sc.js index 4c0494ea0..abe987411 100644 --- a/src/teacher/styledComponents/TextFilterBar.sc.js +++ b/src/teacher/styledComponents/TextFilterBar.sc.js @@ -9,8 +9,10 @@ export const FilterBar = styled(FilterRow).attrs({ $wrap: true })` row-gap: 0.4rem; margin-bottom: 1.75rem; - /* The sort control sits at the far end; no spacer element needed. */ - > *:last-child { + /* A trailing control (My Texts' sort) sits at the far end. Marked explicitly + rather than by :last-child, which flung the last chip across the row on + any bar that has only chips. */ + > .trailing { margin-left: auto; } `; @@ -23,6 +25,16 @@ export const Chip = styled(Tag).attrs({ className: "tiny" })` white-space: nowrap; border-style: ${({ $dashed }) => ($dashed ? "dashed" : "solid")}; + /* $hue/$wash give a class its own colour, so a chip and the tags on the rows + below it read as the same thing. Selected fills with the hue; unselected + is a wash of it, which is alpha over the ground and so works in both + themes. Chips without a hue (All, Not shared) keep the neutral look. */ + ${({ $hue, $wash, $on }) => + $hue && + ($on + ? `border-color: ${$hue}; background-color: ${$hue}; color: white;` + : `border-color: ${$hue}; background-color: ${$wash}; color: ${$hue};`)} + &, &.small { ${({ $on }) => diff --git a/src/utils/misc/classColor.js b/src/utils/misc/classColor.js new file mode 100644 index 000000000..6df124038 --- /dev/null +++ b/src/utils/misc/classColor.js @@ -0,0 +1,45 @@ +import { + tableau_1, tableau_3, tableau_5, tableau_6, +} from "../../components/colors"; + +// From the repo's categorical palette (matplotlib's tab10), the hues that stay +// legible as small text on both the cream and the dark ground. Dropped: grey +// (a class is never "the other one"), red (reads as an error), olive, pink and +// cyan (too pale at this size), orange (the app's own accent). +// +// Six classes is more than anyone here has; beyond that they repeat, which is +// honest -- the colour is a hint on top of the name, never the only signal. +const PALETTE = [ + tableau_1, // blue + tableau_3, // green + tableau_5, // purple + tableau_6, // brown + "#0f766e", // teal + "#9a3412", // burnt orange +]; + +/** + * A stable colour for a class. + * + * Keyed off the class's id rather than its position in a list, so a class keeps + * its colour when the list is filtered, sorted, or a class is added — a colour + * that reshuffles is worse than no colour, because you stop trusting it. + */ +export function classColor(cohortId) { + const key = String(cohortId); + let hash = 0; + for (let i = 0; i < key.length; i++) { + hash = (hash * 31 + key.charCodeAt(i)) % 1000003; + } + return PALETTE[hash % PALETTE.length]; +} + +/** + * The pair a tinted chip needs: the hue for text and border, and a wash of it + * for the fill. The wash is alpha over whatever is behind it, so one value + * works on both the light and the dark ground. + */ +export function classColorTint(cohortId) { + const hue = classColor(cohortId); + return { hue, wash: `color-mix(in srgb, ${hue} 14%, transparent)` }; +} diff --git a/src/teacher/myTextsPage/textFilters.js b/src/utils/misc/classFilters.js similarity index 57% rename from src/teacher/myTextsPage/textFilters.js rename to src/utils/misc/classFilters.js index bfa0a6361..d859247c0 100644 --- a/src/teacher/myTextsPage/textFilters.js +++ b/src/utils/misc/classFilters.js @@ -1,9 +1,14 @@ export const ALL = "all"; export const NOT_SHARED = "not-shared"; -/** Classes a text is shared with, as {id, name}. */ -export function sharedClassesOf(article) { - return article.shared_with || []; +/** + * Which classes a text belongs to, as {id, name}. The teacher's list carries + * `shared_with` (the classes they gave it to); a student's classroom carries + * `from_classes` (the classes it reached them through). Same shape, two names, + * because they answer different questions about the same relationship. + */ +export function classesOf(article) { + return article.shared_with || article.from_classes || []; } /** @@ -12,12 +17,12 @@ export function sharedClassesOf(article) { * texts themselves rather than from the class list, so a chip never promises * texts that are not in the list below it. */ -export function buildClassFilters(articles) { +export function buildClassFilters(articles, { allLabel = "All" } = {}) { const counts = new Map(); let notShared = 0; for (const article of articles) { - const classes = sharedClassesOf(article); + const classes = classesOf(article); if (classes.length === 0) notShared += 1; for (const each of classes) { const existing = counts.get(each.id); @@ -28,7 +33,7 @@ export function buildClassFilters(articles) { const classFilters = [...counts.values()].sort((a, b) => a.name.localeCompare(b.name)); return [ - { id: ALL, name: "All", count: articles.length }, + { id: ALL, name: allLabel, count: articles.length }, ...classFilters, ...(notShared > 0 ? [{ id: NOT_SHARED, name: "Not shared", count: notShared, dashed: true }] : []), ]; @@ -36,6 +41,6 @@ export function buildClassFilters(articles) { export function filterTexts(articles, filterId) { if (filterId === ALL) return articles; - if (filterId === NOT_SHARED) return articles.filter((a) => sharedClassesOf(a).length === 0); - return articles.filter((a) => sharedClassesOf(a).some((each) => each.id === filterId)); + if (filterId === NOT_SHARED) return articles.filter((a) => classesOf(a).length === 0); + return articles.filter((a) => classesOf(a).some((each) => each.id === filterId)); } diff --git a/test/teacher/textFilters.test.js b/test/utils/classFilters.test.js similarity index 75% rename from test/teacher/textFilters.test.js rename to test/utils/classFilters.test.js index afc9eb571..8728ce23b 100644 --- a/test/teacher/textFilters.test.js +++ b/test/utils/classFilters.test.js @@ -3,8 +3,8 @@ import { NOT_SHARED, buildClassFilters, filterTexts, - sharedClassesOf, -} from "../../src/teacher/myTextsPage/textFilters"; + classesOf, +} from "../../src/utils/misc/classFilters"; const text = (id, ...classes) => ({ id, shared_with: classes }); const CUT = { id: 1, name: "CUT Language Centre" }; @@ -57,15 +57,15 @@ describe("filterTexts", () => { }); }); -describe("sharedClassesOf", () => { +describe("classesOf", () => { test("is empty for a text shared with nobody", () => { - expect(sharedClassesOf({})).toEqual([]); + expect(classesOf({})).toEqual([]); }); // The names-only `cohorts` field cannot drive the x on a pill: it carries no // id, so an unshare would post a class name as cohort_id and 401. test("ignores the older names-only field", () => { - expect(sharedClassesOf({ cohorts: ["nana"] })).toEqual([]); + expect(classesOf({ cohorts: ["nana"] })).toEqual([]); }); }); @@ -84,3 +84,20 @@ describe("cohort ids arriving from two endpoints", () => { expect(filters.find((each) => each.name === "nana").count).toBe(2); }); }); + +describe("a student's classroom uses the same filters", () => { + // The teacher's list says which classes a text was shared *with*; the + // student's says which classes it reached them *through*. Same relationship. + const fromClass = (id, name) => ({ id: 1, from_classes: [{ id, name }] }); + + test("chips are built from from_classes too", () => { + const filters = buildClassFilters([fromClass(82, "Danish Class"), fromClass(109, "Aiki")]); + + expect(filters.map((f) => f.name)).toEqual(["All", "Aiki", "Danish Class"]); + }); + + test("no Not-shared chip when every text has a class", () => { + const filters = buildClassFilters([fromClass(82, "Danish Class")]); + expect(filters.some((f) => f.id === NOT_SHARED)).toBe(false); + }); +});