diff --git a/frontends/main/src/app-pages/DashboardPage/Analytics/EngagementTrendChart.tsx b/frontends/main/src/app-pages/DashboardPage/Analytics/EngagementTrendChart.tsx index 647df5d51d..498b99d657 100644 --- a/frontends/main/src/app-pages/DashboardPage/Analytics/EngagementTrendChart.tsx +++ b/frontends/main/src/app-pages/DashboardPage/Analytics/EngagementTrendChart.tsx @@ -2,7 +2,8 @@ import React from "react" import { LineChart } from "@mui/x-charts/LineChart" -import { Skeleton, styled, useTheme } from "ol-components" +import { RiInformationLine } from "@remixicon/react" +import { Skeleton, styled, Tooltip, useTheme } from "ol-components" import type { MonthlyEngagementTrend } from "api/analytics-hooks/organizations" import { EmptyTableMessage, @@ -66,6 +67,34 @@ const TableWrapper = styled.div(({ theme }) => ({ borderTop: `1px solid ${theme.custom.colors.lightGray2}`, })) +const InfoTrigger = styled.span(({ theme }) => ({ + display: "inline-flex", + alignItems: "center", + verticalAlign: "middle", + marginLeft: "4px", + color: theme.custom.colors.silverGrayDark, + cursor: "help", + "& svg": { width: "16px", height: "16px", marginLeft: "4px" }, +})) + +/** + * The desktop header housing the same trigger is `display: none` below `md` + * (`TableHeaderRow`), so this repeats it once, outside the table, in the one + * layout where that header is hidden — never per row, which would turn one + * definition into a repeated focus stop for every month. + */ +const MobileColumnHelp = styled.div(({ theme }) => ({ + display: "none", + [theme.breakpoints.down("md")]: { + display: "flex", + flexWrap: "wrap", + gap: "16px", + marginBottom: "12px", + ...theme.typography.subtitle2, + color: theme.custom.colors.black, + }, +})) + const CHART_HEIGHT = 320 const COLUMN_FLEX = { @@ -82,24 +111,40 @@ const SERIES = [ column: "active", label: "Active learners", color: CATEGORICAL[0], + /** + * Copied from the `monthly_active_learners` field description in + * ol-analytics-api's b2b_dashboard models (mitodl/ol-analytics-api#57), + * itself derived from the backing dbt SQL in ol-data-platform. That + * description lives only in the OpenAPI schema (/openapi.json, /docs) — + * the actual row data this component fetches never carries it, and this + * client is hand-written rather than generated from the schema (see + * the header comment in analytics/types.ts), so there is no fetch-and- + * parse step that could keep this in sync automatically. If the backend + * description changes, this string has to be updated by hand to match. + */ + description: + "Learners who did anything in a course this month: watched a video, attempted a problem, posted in a discussion, used the chatbot, moved through course pages or earned a certificate. Enrolling alone doesn't count. If too few learners were active, the whole month is withheld to avoid identifying them.", }, { key: "new_enrollments", column: "enrollments", label: "New enrollments", color: CATEGORICAL[1], + description: undefined, }, { key: "certificates_earned", column: "certificates", label: "Certificates earned", color: CATEGORICAL[2], + description: undefined, }, ] as const satisfies ReadonlyArray<{ key: keyof MonthlyEngagementTrend column: keyof typeof COLUMN_FLEX label: string color: string + description?: string }> const EngagementTrendChart: React.FC<{ @@ -152,7 +197,16 @@ const EngagementTrendChart: React.FC<{
+ + {SERIES.filter((series) => series.description).map((series) => ( + + {/* eslint-disable-next-line styled-components-a11y/no-noninteractive-tabindex */} + + {series.label} + + + ))} +
@@ -215,6 +283,14 @@ const EngagementTrendChart: React.FC<{ $numeric > {series.label} + {series.description ? ( + + {/* eslint-disable-next-line styled-components-a11y/no-noninteractive-tabindex */} + + + + ) : null} ))} diff --git a/frontends/main/src/app-pages/DashboardPage/Analytics/charts.test.tsx b/frontends/main/src/app-pages/DashboardPage/Analytics/charts.test.tsx index a394a42376..d315010527 100644 --- a/frontends/main/src/app-pages/DashboardPage/Analytics/charts.test.tsx +++ b/frontends/main/src/app-pages/DashboardPage/Analytics/charts.test.tsx @@ -1,5 +1,11 @@ import React from "react" -import { render, screen, within } from "@testing-library/react" +import { + render, + screen, + waitForElementToBeRemoved, + within, +} from "@testing-library/react" +import userEvent from "@testing-library/user-event" import { ThemeProvider, useTheme } from "ol-components" import type { Theme } from "ol-components" import { factories } from "api/analytics-test-utils" @@ -144,6 +150,70 @@ describe("EngagementTrendChart", () => { ).toBeInTheDocument() }) + /** + * "Active learners" alone doesn't say what counts as active — the hover + * icon's accessible label carries the definition for keyboard and screen + * reader users, not just pointer hover. Scoped to the table: `SERIES` with + * the trigger's description also drives a second, mobile-only copy of this + * same trigger outside the table (see the test below), so an unscoped query + * would match both. + */ + test("explains what counts as an active learner from the column header", () => { + renderWithTheme() + + const table = screen.getByRole("table", { name: "Monthly engagement" }) + expect( + within(table).getByLabelText( + /Learners who did anything in a course this month/, + ), + ).toBeInTheDocument() + }) + + /** + * The column header carrying that trigger is hidden below the `md` + * breakpoint (`TableHeaderRow`), so mobile/tablet users need an equivalent + * — once, outside the table, not repeated for every month's row. + */ + test("repeats the active-learner definition once for mobile, outside the table", () => { + renderWithTheme() + + const table = screen.getByRole("table", { name: "Monthly engagement" }) + const triggers = screen.getAllByLabelText( + /Learners who did anything in a course this month/, + ) + expect(triggers).toHaveLength(2) + expect(triggers.some((trigger) => !table.contains(trigger))).toBe(true) + }) + + /** + * A static aria-label proves nothing about whether the `Tooltip` itself + * works — this test would still pass if `Tooltip` were deleted entirely. + * Driving real hover and asserting the rendered popper text catches that; + * keyboard focus isn't asserted here because MUI only opens on focus when + * `:focus-visible` matches (`isFocusVisible`), which this test environment + * doesn't set for a programmatic `.focus()` call — see the same caveat in + * ContractAdminPage.test.tsx's tooltip test. + */ + test("shows the active-learner definition on hover", async () => { + const user = userEvent.setup() + renderWithTheme() + + const table = screen.getByRole("table", { name: "Monthly engagement" }) + const trigger = within(table).getByLabelText( + /Learners who did anything in a course this month/, + ) + + await user.hover(trigger) + expect( + await screen.findByRole("tooltip", { + name: /Learners who did anything in a course this month/, + }), + ).toBeInTheDocument() + + await user.unhover(trigger) + await waitForElementToBeRemoved(() => screen.queryByRole("tooltip")) + }) + test("shows an empty state rather than an empty chart", () => { renderWithTheme() expect(