From 05ff198225ad457011a6445bf328345ad63336f7 Mon Sep 17 00:00:00 2001 From: Mircea Lungu Date: Mon, 31 Aug 2026 19:10:07 +0300 Subject: [PATCH] Let a student filter their classroom by class, with a colour per class With several classes the classroom is a merge, and a term's reading from each adds up -- 49 texts across five classes for the first person to try it. The same chips the teacher gets on My Texts, shown only to a student who has more than one class. The filter helpers move out of teacher/myTextsPage to utils/misc/classFilters, since both sides ask the same question of the same shape: the teacher's list carries `shared_with` (the classes they gave a text to), the student's carries `from_classes` (the classes it reached them through). Each class gets a colour, from the categorical palette already in colors.js (matplotlib's tab10), keyed off the class id rather than its position -- a colour that reshuffles when the list is filtered is worse than no colour. The chip and the tag on every row share it, which is what lets you skim for one class instead of reading each label. Pruned to six hues that stay legible as small text on both grounds: no grey (a class is never "the other one"), no red (reads as an error), no olive, pink or cyan (too pale), no orange (the app's own accent). Beyond six they repeat, which is honest -- the colour is a hint on top of the name, never the only signal. Also fixes the filter bar flinging its last chip across the row: the push-right was on :last-child, which is the sort control on My Texts and a plain chip on a bar that has only chips. Marked with a class instead. Co-Authored-By: Claude Opus 5 --- src/articles/ArticlePreview.js | 8 +++- src/articles/ClassroomArticles.js | 34 +++++++++++++- src/teacher/myTextsPage/AllTexts.js | 14 +++--- .../styledComponents/TextFilterBar.sc.js | 16 ++++++- src/utils/misc/classColor.js | 45 +++++++++++++++++++ .../misc/classFilters.js} | 21 +++++---- .../classFilters.test.js} | 27 ++++++++--- 7 files changed, 142 insertions(+), 23 deletions(-) create mode 100644 src/utils/misc/classColor.js rename src/{teacher/myTextsPage/textFilters.js => utils/misc/classFilters.js} (57%) rename test/{teacher/textFilters.test.js => utils/classFilters.test.js} (75%) 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); + }); +});