diff --git a/docs/A11Y-D01-Accessibility-Baseline_Phase1.md b/docs/A11Y-D01-Accessibility-Baseline_Phase1.md new file mode 100644 index 0000000000..cbb64fca52 --- /dev/null +++ b/docs/A11Y-D01-Accessibility-Baseline_Phase1.md @@ -0,0 +1,358 @@ +# OnTrack Accessibility Baseline - Phase 1 + +**Ticket:** A11Y-D01 - Define the OnTrack accessibility baseline and critical user journeys +**Branch:** `docs/accessibility-baseline` +**Scope:** Documentation and copy specification only. No production code, API behaviour or data flow is changed by this document. +**Source snapshot:** `11.0.x` at `d16f6201caff78082e5e83c540320dbee5871404`. Journey mappings below describe that revision; re-check them when the implementation changes. + +## 1. Purpose & Scope + +The document exists so that every accessibility audit and implementation ticket for OnTrack works from the same test scope, the same severity language, the same evidence format, and a shared understanding of what "done" means for this phase of work. It is written in plain language throughout, so that anyone on the team - _regardless of technical background_ - can read, apply, and rely on it without needing to interpret jargon or infer intent. + +### 1.1 What this document covers + +- Site-wide support for people with disabilities and neurodivergent users, across both technical accessibility (_keyboard, screen reader, semantics, etc._) and cognitive/neurodivergent usability (_plain language, predictable layout, minimal unnecessary complexity_). +- Two critical, end-to-end user journeys - one **student**, one **staff** - mapped against OnTrack's real routes and components, to give future audits a concrete, reproducible starting point rather than an abstract checklist. +- Desktop web, mobile browsers, and the installed standalone PWA defined by [`src/manifest.webmanifest`](../src/manifest.webmanifest). A narrow desktop viewport alone does not establish mobile-app support; use the device and assistive-technology passes in Section 5. + +### 1.2 What this document explicitly excludes + +**Dark mode** is a separate objective and is not covered here. Where this baseline discusses colour and contrast, it is testing OnTrack's existing colour scheme as it stands today, not preparing for or requiring a dark mode implementation. + +### 1.3 Relationship to other objectives and tickets + +- This baseline applies across all objectives - any team running an accessibility audit or building a new feature **should use the terminology, severity scale, and finding template defined here**. +- However, this does **not** make this objective the implementation owner for every feature's accessibility. Individual features and their tickets retain ownership of their own behaviour and their own accessible implementation. This document sets shared expectations; it does not centralise responsibility for meeting them. +- Reuse the existing [A11Y-V01 validation notes](A11Y-V01-validation-notes.md), merged in [PR #239](https://github.com/ontrack-features-t2-2026/doubtfire-web/pull/239), as dated evidence for its recorded build and environments. Those notes document a prior audit and outstanding independent review; this document defines the reusable baseline. Neither document proves that later changes or untested mobile/PWA combinations pass. + +### 1.4 Boundary on future usability + +No one participating in future usability testing or feedback sessions related to this work will be required to disclose a disability or diagnosis in order to take part. Testing methods and recruitment should be designed with this in mind from the outset. + +## 2. Target Standard + +OnTrack's accessibility target for this phase of work is [**WCAG 2.2 Level AA**](https://www.w3.org/TR/WCAG22/). + +This is a **target the team is working toward, not a certification claim**. Adopting this standard means it is the benchmark used to write test methods, judge findings, and prioritise fixes throughout this baseline and the audits that follow it. It does not mean OnTrack currently meets [WCAG 2.2 AA](https://www.w3.org/WAI/WCAG2A-Conformance), that any page has been certified as compliant, or that meeting this target removes the need for ongoing attention as the site changes. Progress toward this target should be understood as an improvement and guardrail program - measurable progress, without ever presenting the current state of the site as fully compliant. + +## 3. Critical User Journeys + +--- + +### 3.1 Student Journey + +Enter Unit - View Task - Submit Work - Find Feedback + +| Step | Route | Component | Role Required | Notes | +| ------------------------------- | -------------------------------------------------- | ---------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| **Enter unit / view task list** | `/projects/:projectId/dashboard` | `ProjectDashboardComponent` | Student (project owner) | Desktop uses a split-pane layout with a CDK drag divider. Below 640 CSS px, use the Overview and Tasks controls; verify keyboard, touch, and screen-reader access to each pane. | +| **View a task (incl. status)** | `/projects/:projectId/dashboard/:taskAbbreviation` | `TaskDashboardComponent`, `TaskStatusCardComponent`, `TaskAssessmentCardComponent` | Student; staff views depend on unit-role checks, with Mod Notes further restricted | Select a task, then Task Details, Task Sheet, or Your Submission. Task Details contains status and, for graded/rated tasks, Assessment Information. Check tab navigation, active-state announcements, and the phone Details control. | +| **Submit work** | N/A — modal opened from a submission action | `UploadSubmissionModalComponent` | Student (project owner) | Group rating (when applicable), upload, then comment. At least 25 trimmed characters are required for Need Help and for feedback/reupload on portfolio-only tasks. Check file selection without dragging, validation, disabled-submit explanation, success/error announcements, and focus return. | +| **Find feedback** | Same selected-task route | `TaskCommentsViewerComponent` in `ProjectDashboardComponent` | Student (project owner) | Read feedback in the comments sidebar, using Open task comments when collapsed below 1000 CSS px. Below 640 CSS px, use the top-level Feedback control. This is separate from the Task Details tab. Verify the correct task's comments, status events, and attachments remain reachable after switching panes. | + +### 3.2 Staff Journey + +Find Submission - Review - Give Feedback - Change Status + +| Step | Route | Component | Role Required | Notes | +| -------------------------------------------- | ----------------------------------------------------------- | --------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| **Find a submission (Inbox)** | `/units/:unitId/tasks/inbox` (+ `/:studentId/:taskDefAbbr`) | `UnitTaskInboxStateComponent` (routeMode: inbox) | Tutor, Convenor, Admin, Auditor | Default landing page for a unit - shared component across all four modes below. | +| **Find a submission (Explorer)** | `/units/:unitId/tasks/definition` (+ variant) | same (routeMode: definition) | Tutor, Convenor, Admin, Auditor | Filter by task definition. | +| **Find a submission (Moderation)** | `/units/:unitId/tasks/moderation` (+ variant) | same (routeMode: moderation) | Tutor, Convenor, Admin, Auditor | Mentor moderation queue. | +| **Find a submission (Overflow)** | `/units/:unitId/tasks/overflow` (+ variant) | same (routeMode: overflow) | Tutor, Convenor, Admin, Auditor | Overdue queue. | +| **Claim an overflow task (when applicable)** | Same selected-task route; footer action | `TaskClaimComponent` in `FooterComponent` | Route whitelist above plus a unit staff role; claim state gates actions | Claim Task is shown for overflow or an already-claimed task. The message says the claim expires after 30 minutes of inactivity. Verify claimed/disabled states and success/error announcements. | +| **Review the submission** | Same selected-task route | `InboxDashboardComponent` | Route whitelist above; staff/Mod Notes have additional component restrictions | Select the submission/document tab and inspect the PDF viewer. On phones, also test Open current document and returning to the selected task. | +| **Give feedback** | Same selected-task route; comments panel | `TaskCommentsViewerComponent`, `TaskCommentComposerComponent` in `InboxComponent` | Authorised staff viewer for the selected task | Open task comments if collapsed; enter and send synthetic feedback. Verify editor naming, send/attachment controls, validation, and the saved comment from the student Feedback pane. | +| **Change task status** | Same selected-task route; footer action | `FooterComponent` calling `Task.updateTaskStatus()` | Route whitelist above; loading, claim ownership, and discussion requirements also gate actions | Use an available status action such as Resubmit, Discuss, or Complete. Check disabled reasons, confirmation dialogs where applicable, success/error announcements, and the resulting student status. In Moderation, use View Status Buttons when necessary. | +| **Grade the submission (when applicable)** | Modal opened during a status change | `GradeTaskModalComponent` via `Task.updateTaskStatus()` | Same action restrictions as the status change | For a graded or quality-rated task, a gradeable target status opens the grade/rating dialog. Check keyboard and screen-reader operation, submit/cancel, focus return, and persisted values after reload. | + +The route guard controls entry, not permission for every mutation; use appropriate synthetic unit-role fixtures and record API denials as well as UI restrictions. Student project access is resolved through the project route, not a Student-only whitelist on the dashboard. + +**Known phone coverage gap at this snapshot:** `InboxComponent` renders `f-footer` only in its desktop branch (600 CSS px and wider). The phone branch contains the document and comments views but no equivalent footer. Do not record claim, status-change, or grade completion on phones as passing without verifying an accessible alternative or a later fix. The journey is mapped here; successful execution on every device remains audit work. + +## 4. Accessibility Test Areas + +--- + +For each area: _what to check_, _how to check it_, and _what evidence to capture_. Where the journey mapping above already surfaced a specific known risk, it's referenced directly so testers know exactly where to start. These areas are the minimum journey checks, not an exhaustive WCAG conformance checklist. + +### 4.1 Keyboard + +Every interactive element (buttons, tabs, modals, form controls) must be reachable and operable using only a keyboard. + +- **Known risk**: The student dashboard's task-list resize divider (`ProjectDashboardComponent`) is currently mouse-drag only (CDK drag-drop) — check whether a keyboard-accessible alternative exists. +- **Method**: Navigate each journey step using Tab/Shift+Tab/Enter/Space/Arrow keys only, no mouse. +- **Evidence**: Screen recording of keyboard-only traversal, noting any unreachable or un-triggerable controls. + +### 4.2 Focus + +Focus should be visible and not obscured, and should move logically. A modal may contain focus while open, but must provide an operable exit. + +- **Known risk**: Modals (`GradeTaskModalComponent`, `UploadSubmissionModalComponent`, `BatchFeedbackWorkflowDialogComponent`) should trap focus while open and return it sensibly on close. +- **Method**: Open/close each modal and confirm focus placement before, during, and after; check focus order across multi-stage flows (e.g. the upload modal's group - details - comments stages). +- **Evidence**: Screen recording or annotated screenshots showing focus indicator position at each step. + +### 4.3 Screen Reader + +All content and state changes must be announced correctly (NVDA/VoiceOver, per the environment matrix in Section 5). + +- **Known risks**: Success messages shown via alert + snack bar (task claiming); tab switching in `TaskDashboardComponent`/`InboxDashboardComponent`; loading states (`inboxLoading` in the staff inbox); the redirect to `/unauthorised` when a role-restricted route is accessed without permission (`role-whitelist.guard.ts`) - confirm the redirect and resulting page are announced clearly, and that the brief loading check beforehand doesn't leave a screen reader user on a blank or ambiguous page. +- **Method**: Navigate each journey step with a screen reader active; confirm dynamic changes (loading, success/error, tab switches, redirects) are announced, not just visually shown. +- **Evidence**: screen recording with screen reader audio, or a transcript of announcements against expected announcements. + +### 4.4 Semantics + +Correct use of headings, landmarks, lists, buttons vs. links, and ARIA roles/attributes where native HTML isn't sufficient. + +- **Method**: Inspect the rendered DOM (browser dev tools or an automated tool) for each journey step. +- **Evidence**: Annotated DOM snapshot or automated scan report per page/step. + +### 4.5 Zoom / Text Resize + +Content must remain usable at 200% browser zoom and at increased OS/browser text-size settings, with no loss of content or function. + +- **Method**: Test each journey step at 200% zoom and at a large text-size setting. +- **Evidence**: Before/after screenshots at default and 200% zoom. + +### 4.6 Reflow + +For vertically scrolling content, test at **320 CSS px wide**, equivalent to a 1280 CSS px viewport at 400% browser zoom. Content and controls must remain available without scrolling in two dimensions. [WCAG 2.2 SC 1.4.10](https://www.w3.org/WAI/WCAG22/Understanding/reflow.html) allows exceptions for content that needs a two-dimensional layout, such as some tables or diagrams; record the specific exception rather than exempting the whole page or its controls. + +- **Known risk**: Student pane switching and collapsed comments; staff actions missing from the phone branch (Section 3.2); PDF controls, modals, and the on-screen keyboard covering feedback controls. +- **Method**: Complete each journey at 320 CSS px and at each device viewport in Section 5, including 400% desktop zoom. Check around the staff 600px and student 640px layout boundaries, and the 1000px comments boundary. Test portrait and landscape, including with the on-screen keyboard open. +- **Evidence**: Screenshots or recordings with CSS viewport dimensions, zoom level, orientation, and any justified two-dimensional-content exception recorded. + +### 4.7 Contrast + +Text and meaningful UI elements must meet **WCAG 2.2 AA** contrast ratios (4.5:1 for normal text, 3:1 for large text/UI components) checked against whichever colour theme/mode is active (excluding dark mode itself, which is out of scope). + +- **Method**: Automated contrast check per page/step. +- **Evidence**: Contrast checker output per checked element. + +### 4.8 Colour + +Information must never be conveyed by colour alone (e.g. status icons/labels in `taskStatusData`, warning/overflow icons in the staff task list). + +- **Method**: Review each status/warning indicator and confirm an icon, label, or text equivalent accompanies any colour coding. +- **Evidence**: Screenshot with colour-blindness simulation applied (e.g. a simulator extension), or annotated screenshot noting the non-colour cue. + +### 4.9 Motion + +As a product expectation, non-essential animation should respect the OS/browser "reduce motion" setting. Also check applicable WCAG requirements for flashing and moving content; enabling reduced motion alone does not establish that those requirements pass. + +- **Method**: Enable "reduce motion" at the OS level and repeat each journey step, checking nothing becomes unusable. +- **Evidence**: Screen recording with reduce-motion enabled. + +### 4.10 Cognitive Clarity + +Plain language, predictable layout, clear error/validation messaging, and minimal unnecessary complexity. + +- **Known risk**: The upload submission modal's minimum comment length requirement (25+ characters) and its disabled-submit state - confirm the reason for disablement is clearly communicated, not just visually implied. +- Method: walk through each journey step as a first-time user would, noting any unclear instructions, ambiguous error states, or unexplained disabled controls. +- Evidence: written notes per step, plus screenshots of any unclear or ambiguous UI encountered. + +## 5. Test Environment Matrix + +--- + +The matrix below defines repeatable coverage. Record exact browser, OS, assistive-technology, and app-build versions; "latest" alone is not reproducible. Mark an unavailable combination **Not tested**, with an owner for follow-up, rather than inferring a pass from another device. + +### Browsers + +- Chrome (latest stable) - **Primary** +- Firefox (latest stable) +- Safari (latest stable, macOS) / Edge (latest stable, Windows) + +### Operating Systems + +- Windows (latest supported release) +- macOS (latest supported release) +- iOS and Android for the mobile-browser and installed-PWA passes below. Record any unavailable platform as a coverage gap. + +### Screen Readers + +- NVDA (Windows) - **Primary**, (paired with Chrome or Firefox) +- VoiceOver (macOS) - (paired with Safari) +- VoiceOver with Safari on iOS; TalkBack with Chrome on Android. Repeat the journeys in the installed standalone PWA on each available platform, using touch exploration and swipe navigation. +- Additional, as available: JAWS (Windows). + +### Viewports + +- Desktop: 1920×1080 or 1366×768 CSS px, plus 200% text/zoom and the 400% reflow pass described in Section 4.6. +- Reflow minimum: 320 CSS px wide, independent of whichever phone is available. +- Mobile: 390×844 CSS px as a repeatable browser fixture, plus the actual iOS/Android device dimensions in portrait and landscape. Responsive emulation complements physical-device testing; it does not replace it. + +### Mobile app / installed PWA + +- Run each journey in a normal mobile browser and after launching the installed OnTrack PWA. Confirm the same build is loaded; a service worker can serve an older bundle (see [service-worker guidance](service-worker.md)). +- Include launch/resume, returning from the file picker or document viewer, browser/device back navigation, safe-area insets, focus after pane changes, touch target usability, large text, and the on-screen keyboard. Verify the selected task survives these transitions and feedback/submission actions remain reachable. +- Record browser versus standalone display mode in every finding. A browser pass must not be reported as an installed-app pass. Document missing staff phone actions from Section 3.2 as an open issue until verified fixed. + +Each finding logged against the finding template (Section 7) should record which specific browser/OS/screen-reader/viewport combination it was found in, since accessibility issues can be combination-specific. + +## 6. Severity Scale (P0–P3) + +--- + +Each finding is assigned one severity level, based on a combination of four factors: + +- How much harm or exclusion it causes. +- Whether it blocks a task entirely. +- How often a user would hit it. +- Whether it affects a shared component used across many pages. + When factors point to different levels, use the **highest** applicable severity. + +### P0 — Blocking + +A user relying on assistive technology or an accessibility feature **cannot complete a core task at all** (e.g. cannot submit work, cannot find their grade, cannot navigate past a certain point). No workaround exists within the page. + +- **Example**: A modal traps keyboard focus with no way to close or continue. +- **Shared-component impact**: A shared component that blocks a core task with no accessible workaround is P0 across its affected views. Reuse alone does not turn a minor issue into a blocker. + +### P1 — Severely Degraded + +A core task is **technically possible but significantly harder, slower, or more frustrating** for a user of assistive technology or an accessibility feature - not fully blocked, but a serious barrier. + +- **Example**: A status change is made but not announced to screen reader users, forcing them to guess or re-check manually. +- **Example**: Content critical to a decision (e.g. feedback text) is present but not reachable in a logical reading/focus order. + +### P2 — Minor Barrier + +The task is completable without serious difficulty, but there's a **real, noticeable accessibility problem** - inconsistent focus indication, a contrast ratio just below AA, a redundant or confusing label. + +- **Example**: A warning icon relies on colour alone but a text label is present elsewhere on the same row (so the information isn't entirely lost, just less discoverable). + +### P3 — Cosmetic / Low Impact + +A **minor inconvenience** with negligible effect on task completion - small inconsistencies, non-critical polish issues, or edge cases affecting very few users or very rarely encountered states. + +- **Example**: a decorative element lacks `alt=""` but conveys no information. + +### Applying the scale + +- **Harm**: How seriously does this affect a real user's ability to use OnTrack independently and with dignity? +- **Task-blocking**: Does it stop a task outright, slow it down, or barely register? +- **Frequency**: Is this hit on every use of a page, or only in a rare edge case? +- **Shared-component impact**: Record all affected views and increase triage priority when an issue is widespread. Change severity only when the broader evidence meets that level's harm and task-blocking criteria. + +## 7. Finding Template + +--- + +Every accessibility finding logged against this baseline uses the same fields, in the same structure, regardless of who logs it or which journey it comes from. Do not add, remove, or rename fields per-ticket - the structure only stays reusable if it stays fixed. + +| Field | Description | +| ------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| **Route** | The exact URL/path where the issue was found (e.g. `/units/:unitId/tasks/inbox`). Use the real path pattern from the app, not a paraphrase. | +| **Component** | The Angular **component name** responsible for the affected UI (e.g. `GradeTaskModalComponent`), if known. Helps whoever fixes the issue locate the code quickly. | +| **Steps** | The exact sequence of actions to reproduce the issue, written so someone unfamiliar with the finding could follow them without guessing (include role/account used, browser/OS/screen reader, and viewport - matching an entry from **Section 5**'s matrix). | +| **Expected Result** | What should happen, per **WCAG 2.2 AA** and/or this baseline's test-area guidance (**Section 4**). | +| **Actual Result** | What actually happens - described factually, without severity judgement built into the wording. | +| **Severity** | One of **P0–P3**, assigned per the criteria in **Section 6**. | +| **Evidence** | A screen recording, screenshot, or tool output (e.g. software generated report, contrast checker result) demonstrating the issue. Use demonstration accounts and sanitised data only (see **Section 8**). | +| **Owner** | The person or team responsible for triaging or fixing the finding. | +| **Re-test Status** | Not yet re-tested / Fixed - confirmed / Fixed - not confirmed / Still present / Won't fix (with reason). Updated after a fix is attempted, using the same environment the issue was originally found in wherever possible. | + +## 8. Security & Privacy Notes + +> **Use demonstration accounts and sanitised evidence only.** No real student, staff, or unit data should appear in any screenshot, recording, or written finding produced during audits against this baseline. + +> **Do not use real student assessment content in test-plan examples.** Any example task, submission, or feedback used to illustrate a journey or a finding must be fabricated or clearly marked as a demonstration. + +> **Hidden accessibility text is still data exposure.** Content added purely for assistive technology - `aria-label`, `alt` text, visually-hidden spans, and similar - must be checked for unauthorised information the same way visible content is. A hidden label is not a safe place to leak information that shouldn't be shown to the current viewer (e.g. another student's name, an internal-only note, a role-restricted detail). This applies equally to elements found during the journeys above, such as the role-gated Tutor Notes tab (`InboxDashboardComponent`, `TaskDashboardComponent`) - hidden text near those areas should be checked as carefully as the visible UI already is. + +## 9. Phase 1 — Definition of Done + +### 9.1 What Phase 1 explicitly does not mean + +- That any accessibility audit has actually been run - running the audits is separate, future work. +- That any component has been fixed, or that any specific page currently passes **WCAG 2.2 AA**. +- That OnTrack is accessibility-compliant or certified in any formal sense. **WCAG 2.2 AA** is adopted here as a **target to work toward**, not a claim being made about the current state of the site. + +### 9.2 Explicitly out of scope for Phase 1 + +(per this ticket): Running the audits, fixing flagged components, implementing dark mode, purchasing commercial accessibility tooling, and formal legal certification. These are future work, to be scoped as separate tickets once this baseline is in use. + +### 9.3 Known open items + +For whoever picks up the next piece of work. + +- The staff phone branch omits the footer used for claims, status changes, and grading (Section 3.2). Audit and resolve this separately before claiming end-to-end staff mobile support. +- Execute the desktop, mobile-browser, and installed-PWA matrix in Section 5 against the chosen integration build. The source mapping in this document is not evidence of a passing runtime audit. +- Re-test the observations and outstanding independent review in A11Y-V01 where relevant; retain their original evidence and build context rather than duplicating or silently declaring them resolved. + +## 10 Phase 1 (this baseline) is complete when + +| Requirement | Result | Evidence | +| --------------------------------------------------------------------------------------------------------------------- | ----------------------------- | ----------- | +| The target standard and scope are written in plain language. | **Met** | Section 1 | +| The plan covers both technical accessibility and cognitive or neurodivergent usability. | **Met** | Section 1.1 | +| Dark mode is clearly excluded from implementation while contrast and colour use remain testable. | **Met** | Section 1.2 | +| At least one student journey and one staff journey are mapped from start to finish, with known device gaps recorded. | **Met (source mapping only)** | Section 3 | +| Every accessibility test area has a repeatable method and a defined evidence type. | **Met** | Section 4 | +| The test environment matrix exists that the team can actually reproduce. | **Met** | Section 5 | +| The severity scale distinguishes a blocked task from a minor inconvenience. | **Met** | Section 6 | +| The findings template exists and can be reused without editing its structure. | **Met** | Section 7 | +| Security and privacy expectations for future audit evidence are documented. | **Met** | Section 8 | +| The document states that Phase 1 is an improvement and guardrail program, not proof that the whole site is compliant. | **Met** | Section 9.1 | + +## 11. Known Codebase Notes + +--- + +### AngularJS + +- Angular migration in progress - legacy `.coffee` state files and orphaned `.tpl.html`/`.scss` files still exist alongside migrated `.component.ts` files (e.g. `dashboard.tpl`, `task-dashboard.tpl`). Not active code, but can be confusing when browsing. +- Route-vs-template mismatches to expect - old CoffeeScript state definitions may reference templates that no longer exist post-migration. + +### Confirmations + +- What initially looked like a role-guard inconsistency between the bare `/units/:unitId/tasks` path and `tasks/inbox`/`tasks/definition`/etc. is **not an inconsistency** - they are two different features sharing a URL prefix. The bare `tasks` path renders `TaskViewerStateComponent`, a task-_definition_ viewer (unit-administration-adjacent, correctly restricted to Convenor/Admin/Auditor). The `tasks/inbox` etc. paths render `UnitTaskInboxStateComponent`, which reviews individual student _submissions_ (correctly includes Tutor, since that's core marking work). +- `SelectedTaskService` is shared infrastructure between staff and student views - task-selection state isn't duplicated per role. + +### Status and feedback paths + +- Student submission processing calls `Task.processTaskStatusChange()` from `UploadSubmissionModalComponent`; the method belongs to the task model. +- Staff footer actions call `Task.updateTaskStatus()`, which may open `GradeTaskModalComponent` before persisting a gradeable status. Staff feedback uses the shared comments viewer and composer, while student grade/quality summaries appear in `TaskAssessmentCardComponent`. + +## 12 Coverage + +--- + +### 12.1 Files + +| | | +| -------------------------------------- | ------------------------------------------------------------------------------------ | +| `unit-task-inbox-state.component.ts` | Confirmed the shared staff inbox/explorer/moderation/overflow component | +| `app.routes.ts` | The authoritative routing file - confirmed all staff route paths | +| `inbox-dashboard.component.ts` | Confirmed the read-only submission viewer | +| `inbox-dashboard.component.html` | Confirmed no grading action lived there | +| `staff-task-list.component.ts` | Confirmed task selection, keyboard shortcuts, filtering | +| `task-claim.component.ts` | Confirmed the claim action | +| `task-status.ts` | Confirmed the status vocabulary | +| `grade-task-modal.component.ts` | Confirmed the actual grading action | +| `project-dashboard.component.ts` | Confirmed the student split-pane layout | +| `task-dashboard.component.ts` | Confirmed task tabs and staff-view restrictions; comments are outside this component | +| `upload-submission-modal.component.ts` | Confirmed the student submission action | +| `role-whitelist.guard.ts` | Confirmed guard redirect behaviour | +| `task-viewer-state.component.ts` | Resolved the apparent role-guard inconsistency | +| `definitions.coffee` | Legacy staff route definition (superseded) | + +The completed journey mapping also traces these active sources (paths are relative to this document): + +- [Project dashboard template](../src/app/projects/states/dashboard/project-dashboard/project-dashboard.component.html): phone Feedback control and desktop comments sidebar. +- [Staff inbox template](../src/app/units/states/tasks/inbox/inbox.component.html): document/comments views and the desktop-only footer placement. +- [Footer template](../src/app/common/footer/footer.component.html) and [component](../src/app/common/footer/footer.component.ts): claim visibility, status actions, and their enabling conditions. +- [Task model](../src/app/api/models/task.ts): `updateTaskStatus()`, grade-dialog dispatch, persistence, and status-change processing. +- [Comments viewer](../src/app/tasks/task-comments-viewer/task-comments-viewer.component.html) and [composer](../src/app/tasks/task-comment-composer/task-comment-composer.component.ts): feedback display and submission. +- [Assessment card](../src/app/projects/states/dashboard/directives/task-dashboard/directives/task-assessment-card/task-assessment-card.component.html): student grade and quality-point display. + +### 12.2 Paths + +| | | +| --------------------------------------- | ----------------------------------------------------------------- | +| `units/states/tasks/inbox/` | Revealed the Angular migration | +| `units/states/tasks/inbox/directives/` | Revealed inbox-dashboard, moderation, staff-task-list, task-claim | +| `projects/states/dashboard/` | Revealed directives, dashboard.tpl, selected-task.service | +| `projects/states/dashboard/directives/` | Revealed progress-dashboard, student-task-list, task-dashboard | +| `task-dashboard/` | Revealed `task-dashboard.component.ts` plus legacy files | diff --git a/docs/accessibility/AUTHORING.md b/docs/accessibility/AUTHORING.md new file mode 100644 index 0000000000..4bb7bede27 --- /dev/null +++ b/docs/accessibility/AUTHORING.md @@ -0,0 +1,86 @@ +# Accessibility contribution guide + +Use this guide for frontend changes and reviews. WCAG 2.2 AA is the improvement target, not a claim of compliance. Start with the [merged A11Y-D01 baseline](../A11Y-D01-Accessibility-Baseline_Phase1.md), the [manual regression pack](REGRESSION.md), and the [remediation and handover register](REMEDIATION.md). The register distinguishes merged code, changes awaiting review, and checks still needing a person. + +For ticket acceptance and reviewer handover, follow the [closure guide](CLOSURE.md) and [run record](RUN-RECORD.md). The [A01 audit](evidence/a01/A01-LINT-AUDIT.md) records the main-branch findings and the fixed-stack scan separately. + +## Controls and structure + +- Use ` @@ -129,6 +133,7 @@

Teaching Breaks for {{ newOrSelectedTeachingPeriod.name }}

 - @@ -158,7 +167,7 @@

Edit Feedback Templates for Outcom @if (selectedTemplate) {
-

Edit Template

+

Edit Template

@if (selectedTemplate.isNew) { diff --git a/src/app/common/learning-outcome-editor/learning-outcome-editor.component.html b/src/app/common/learning-outcome-editor/learning-outcome-editor.component.html index c8c668aabe..8ebac4bb79 100644 --- a/src/app/common/learning-outcome-editor/learning-outcome-editor.component.html +++ b/src/app/common/learning-outcome-editor/learning-outcome-editor.component.html @@ -1,202 +1,295 @@
- - - - - - - - - - - - - - - - - - - - - - - - -
Abbreviation - {{ learningOutcome.abbreviation }} - Short Description - {{ learningOutcome.shortDescription }} - Full Outcome Description - {{ learningOutcome.fullOutcomeDescription }} - Connected Learning Outcomes - @for (outcome of getLinkedOutcomes(learningOutcome); track outcome.abbreviation) { - {{ - outcome.abbreviation - }} - } - - @if (learningOutcomeHasChanges(learningOutcome)) { - - } - -
- - -
- - - + +
- - - @if (abbreviationPrefix !== 'GLO') { - - + + } + - - - } - - - - - - + + + +
+ + + + + + + + + + + + + + + + + + + + + + + + + + + +
Code + + + Short description + {{ learningOutcome.shortDescription }} + Full outcome + {{ learningOutcome.fullOutcomeDescription }} + Connected to + + @for (outcome of getLinkedOutcomes(learningOutcome); track outcome.id) { + + {{ outcome.abbreviation }} + + } + + Actions +
+ @if (learningOutcomeHasChanges(learningOutcome)) { + + } + +
+
+ @if (filtering) { + + } @else { + + } +
+
+ + + @if (selectedOutcome) { -
-
-

Edit Outcome

+
+
+

+ {{ selectedOutcome.isNew ? 'New outcome' : 'Edit ' + selectedOutcome.abbreviation }} +

-
- - Abbreviation - - +
+
+ + + + Up to 5 characters. + +
- - Short Description - + + + + +
+
+ +
+ + +
- - Full Outcome Description - - - @if (abbreviationPrefix !== 'GLO') { - - Connected Learning Outcomes - - @for (outcome of selectedConnectedOutcomes(); track outcome.abbreviation) { - - {{ outcome.abbreviation }} - - - } - - - - @for (outcome of filteredOutcomes(); track outcome) { - {{ outcome.abbreviation }} - {{ outcome.shortDescription }} - } - - +
+ + + + @for (outcome of selectedConnectedOutcomes(); track outcome.abbreviation) { + + {{ outcome.abbreviation }} + + + } + + + + @for (outcome of filteredOutcomes(); track outcome.id) { + + {{ outcome.abbreviation }} · {{ outcome.shortDescription }} + + } + + + {{ + abbreviationPrefix === 'TLO' + ? 'Type a code to link unit or institution outcomes this task works towards.' + : 'Type a code to link institution outcomes this one works towards.' + }} + + +
} - -
- - -
-
+ +
+ + +
+ + +
+

+ {{ data.externalName }} + @if (data.externalName === 'Doubtfire') { + Version 6 + } +

+

Task-based learning, feedback and portfolio assessment.

+
+ + + +
+

+ + Lead contributors +

+
    + @for (person of data.mainContributors; track person.login; let i = $index) { +
  • + +
    + {{ person.name || person.login }} + +
    + @if (person.html_url) { + + + + } +
    • + } +
+
+ +
+

+ + Contributors +

+
+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
Contributor + + + + + Contributions{{ element.totalContributions }}API{{ element.apiContributions }}Web{{ element.webContributions }}Deploy{{ element.deployContributions }}doubtfire.io{{ element.ioContributions }}
+ +
+
+
+ +
+

+ + Open source +

+
+

+ {{ data.externalName }} is licensed under the GNU Affero General Public License (AGPL) v3.0. + © 2012–2023. Made in Melbourne. +

+
+ + + Organisation + + + + Star on GitHub + +
+
+
+ + + + + diff --git a/src/app/common/modals/about-doubtfire-modal/about-doubtfire-modal-content.component.spec.ts b/src/app/common/modals/about-doubtfire-modal/about-doubtfire-modal-content.component.spec.ts new file mode 100644 index 0000000000..3b88740511 --- /dev/null +++ b/src/app/common/modals/about-doubtfire-modal/about-doubtfire-modal-content.component.spec.ts @@ -0,0 +1,77 @@ +import {beforeEach, describe, expect, it} from 'vitest'; +import {NO_ERRORS_SCHEMA} from '@angular/core'; +import {ComponentFixture, TestBed} from '@angular/core/testing'; +import {MAT_DIALOG_DATA} from '@angular/material/dialog'; +import {AboutDialogData} from './about-dialog-data'; +import {AboutDoubtfireModalContent} from './about-doubtfire-modal.component'; + +describe('AboutDoubtfireModalContent', () => { + let fixture: ComponentFixture; + let data: AboutDialogData; + + beforeEach(async () => { + data = new AboutDialogData(); + data.externalName = 'OnTrack'; + data.mainContributors = [ + { + login: 'macite', + name: 'Andrew Cain', + avatar_url: 'https://example.test/macite.png', + html_url: 'https://github.com/macite', + }, + { + login: 'alexcu', + name: 'Alex Cummaudo', + avatar_url: 'https://example.test/alexcu.png', + html_url: 'https://github.com/alexcu', + }, + { + login: 'jakerenzella', + name: undefined, + avatar_url: '/assets/images/person-unknown.gif', + html_url: undefined, + }, + ]; + + await TestBed.configureTestingModule({ + declarations: [AboutDoubtfireModalContent], + providers: [{provide: MAT_DIALOG_DATA, useValue: data}], + schemas: [NO_ERRORS_SCHEMA], + }).compileComponents(); + + fixture = TestBed.createComponent(AboutDoubtfireModalContent); + fixture.detectChanges(); + }); + + const cards = () => + Array.from(fixture.nativeElement.querySelectorAll('.about__card')) as HTMLElement[]; + + it('renders a card with a name and a named avatar for each lead contributor', () => { + const rendered = cards(); + expect(rendered).toHaveLength(3); + + const expected = ['Andrew Cain', 'Alex Cummaudo', 'jakerenzella']; + rendered.forEach((card, i) => { + const name = card.querySelector('.about__person-name') as HTMLElement; + const img = card.querySelector('img') as HTMLImageElement; + expect(name.textContent.trim()).toBe(expected[i]); + expect(img.getAttribute('alt')).toBe(expected[i]); + }); + }); + + it('labels profile links and omits them until a profile URL is known', () => { + const [andrew, , jake] = cards(); + const link = andrew.querySelector('a.about__icon-link') as HTMLAnchorElement; + expect(link.getAttribute('href')).toBe('https://github.com/macite'); + expect(link.getAttribute('aria-label')).toContain('Andrew Cain on GitHub'); + expect(jake.querySelector('a.about__icon-link')).toBeNull(); + }); + + it('keeps a close action in the dialog footer', () => { + const close = fixture.nativeElement.querySelector( + 'mat-dialog-actions button', + ) as HTMLButtonElement; + expect(close.textContent.trim()).toBe('Close'); + expect(close.hasAttribute('mat-dialog-close')).toBe(true); + }); +}); diff --git a/src/app/common/modals/about-doubtfire-modal/about-doubtfire-modal-content.tpl.html b/src/app/common/modals/about-doubtfire-modal/about-doubtfire-modal-content.tpl.html deleted file mode 100644 index 4d98486b5f..0000000000 --- a/src/app/common/modals/about-doubtfire-modal/about-doubtfire-modal-content.tpl.html +++ /dev/null @@ -1,96 +0,0 @@ - -
- OnTrack logo -

- {{data.externalName}} - - Version 6 - -

-
- -
-

Lead Contributors

- -

Contributors

- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
Contributor - - @{{element.login}} - - Contributions{{element.totalContributions}}API{{element.apiContributions}}Web{{element.webContributions}}Deploy{{element.deployContributions}}doubtfire.io{{element.ioContributions}}
-

© 2012–2023. Made in Melbourne.

-

- OnTrack is an - Open Source - project licensed under GNU Affero General Public License (AGPL) v3.0. Star us - on - GitHub! -

-
- - - - diff --git a/src/app/common/modals/about-doubtfire-modal/about-doubtfire-modal.component.ts b/src/app/common/modals/about-doubtfire-modal/about-doubtfire-modal.component.ts index e0cdf53f73..e50cbb4d79 100644 --- a/src/app/common/modals/about-doubtfire-modal/about-doubtfire-modal.component.ts +++ b/src/app/common/modals/about-doubtfire-modal/about-doubtfire-modal.component.ts @@ -11,7 +11,8 @@ import {GithubProfile} from './github-profile'; @Component({ selector: 'about-doubtfire-dialog', - templateUrl: 'about-doubtfire-modal-content.tpl.html', + templateUrl: 'about-doubtfire-modal-content.component.html', + styleUrls: ['about-doubtfire-modal.scss'], changeDetection: ChangeDetectionStrategy.Eager, standalone: false, }) @@ -35,7 +36,7 @@ export class AboutDoubtfireModalContent { /** * The about doubtfire modal service - used to create and show the modal */ -// eslint-disable-next-line max-classes-per-file + @Injectable() export class AboutDoubtfireModal { private loaded: boolean; diff --git a/src/app/common/modals/about-doubtfire-modal/about-doubtfire-modal.scss b/src/app/common/modals/about-doubtfire-modal/about-doubtfire-modal.scss index e69de29bb2..8bf0ef959d 100644 --- a/src/app/common/modals/about-doubtfire-modal/about-doubtfire-modal.scss +++ b/src/app/common/modals/about-doubtfire-modal/about-doubtfire-modal.scss @@ -0,0 +1,397 @@ +// About dialog. Theme tokens only, so light and dark both follow the resolved theme. +// Spacing sits on a 24px dialog gutter with 16px and 8px steps inside it. + +$ease-out: cubic-bezier(0.23, 1, 0.32, 1); +$gutter: 24px; + +:host { + // Material renders the dialog host as display: contents, so it is left alone and + // only carries inherited values. + color: var(--ot-color-text); + + // The dark border token sits just above the surface, so it reads as an edge. In + // light the same token is a strong grey, so the card edge is softened towards it. + --about-card-border: color-mix(in srgb, var(--ot-color-border) 45%, var(--ot-color-surface)); +} + +:host-context([data-ot-theme='dark']) { + --about-card-border: var(--ot-color-border); +} + +// --- Header --------------------------------------------------------------- + +.about__header { + display: flex; + align-items: center; + gap: 16px; + padding: $gutter $gutter 16px; +} + +.about__logo { + flex: none; + width: 40px; + height: 40px; +} + +.about__identity { + min-width: 0; +} + +// Material's dialog title brings its own padding and a 40px baseline strut. The +// header owns the spacing here, so both are cleared. +.about__name.mat-mdc-dialog-title { + display: flex; + flex-wrap: wrap; + align-items: baseline; + gap: 4px 8px; + margin: 0; + padding: 0; + font-size: 1.375rem; + font-weight: 600; + line-height: 1.25; + letter-spacing: -0.01em; + color: var(--ot-color-text); + + &::before { + display: none; + } +} + +.about__version { + font-size: 0.8125rem; + font-weight: 500; + letter-spacing: 0; + color: var(--ot-color-text-muted); +} + +.about__tagline { + margin: 2px 0 0; + font-size: 0.875rem; + line-height: 1.4; + color: var(--ot-color-text-muted); +} + +// --- Body ----------------------------------------------------------------- + +.about__content.mat-mdc-dialog-content { + display: flex; + flex-direction: column; + gap: $gutter; + padding: 8px $gutter $gutter; + color: var(--ot-color-text); + container-type: inline-size; +} + +.about__section { + display: flex; + flex-direction: column; + gap: 12px; + min-width: 0; +} + +.about__label { + display: flex; + align-items: center; + gap: 8px; + margin: 0; + font-size: 0.875rem; + font-weight: 600; + line-height: 1.4; + color: var(--ot-color-text); +} + +.about__label-icon { + width: 18px; + height: 18px; + font-size: 18px; + color: var(--ot-color-text-muted); +} + +// --- Lead contributor cards ----------------------------------------------- + +.about__lead-grid { + display: grid; + grid-template-columns: minmax(0, 1fr); + gap: 12px; + margin: 0; + padding: 0; + list-style: none; +} + +@container (min-width: 440px) { + .about__lead-grid { + grid-template-columns: repeat(2, minmax(0, 1fr)); + } +} + +@container (min-width: 640px) { + .about__lead-grid { + grid-template-columns: repeat(3, minmax(0, 1fr)); + } +} + +.about__card { + display: flex; + flex-direction: column; + align-items: center; + gap: 12px; + min-width: 0; + padding: 20px 16px 16px; + border: 1px solid var(--about-card-border); + border-radius: var(--ot-radius-md); + background: var(--ot-color-surface); + text-align: center; + animation: about-card-in 200ms $ease-out both; + animation-delay: calc(var(--about-i, 0) * 50ms); +} + +@keyframes about-card-in { + from { + opacity: 0; + transform: translateY(8px); + } +} + +.about__avatar { + display: block; + width: 80px; + height: 80px; + border-radius: var(--ot-radius-circle); + object-fit: cover; + background: var(--ot-color-surface-raised); + box-shadow: + 0 0 0 3px var(--ot-color-surface), + 0 0 0 4px var(--about-card-border); + transition: transform 200ms $ease-out; +} + +@media (hover: hover) and (pointer: fine) { + .about__card:hover .about__avatar { + transform: translateY(-2px); + } +} + +.about__person { + display: flex; + flex-direction: column; + gap: 2px; + min-width: 0; + max-width: 100%; +} + +.about__person-name { + font-size: 1rem; + font-weight: 600; + line-height: 1.3; + color: var(--ot-color-text); + overflow-wrap: anywhere; + text-wrap: balance; +} + +.about__person-login { + font-size: 0.8125rem; + line-height: 1.4; + color: var(--ot-color-text-muted); + overflow-wrap: anywhere; +} + +// A small outlined icon button, pinned to the card foot so buttons line up across +// cards of equal height. +.about__icon-link.mat-mdc-icon-button { + --mat-icon-button-state-layer-size: 36px; + --mat-icon-button-icon-size: 18px; + margin-top: auto; + width: 36px; + height: 36px; + padding: 8px; + border: 1px solid var(--about-card-border); + border-radius: var(--ot-radius-circle); + color: var(--ot-color-text-muted); + + .mat-icon { + width: 18px; + height: 18px; + font-size: 18px; + line-height: 18px; + } +} + +// One column: a compact row per person, so a phone does not stack three tall cards. +// The link sits under the name so the name keeps the full text column and wraps +// only at a space. +@container (max-width: 439px) { + .about__card { + display: grid; + grid-template-columns: 72px minmax(0, 1fr); + align-items: center; + column-gap: 16px; + row-gap: 8px; + padding: 16px; + text-align: start; + } + + .about__avatar { + grid-row: span 2; + width: 72px; + height: 72px; + } + + .about__person { + align-self: end; + } + + .about__person-name, + .about__person-login { + overflow-wrap: break-word; + } + + .about__icon-link.mat-mdc-icon-button { + align-self: start; + justify-self: start; + margin-top: 0; + } +} + +// --- Contributors table --------------------------------------------------- + +.about__table-frame { + overflow-x: auto; + border: 1px solid var(--about-card-border); + border-radius: var(--ot-radius-md); + background: var(--ot-color-surface); +} + +// The frame scrolls sideways on wider dialogs, so it takes keyboard focus as a region. +.about__table-frame:focus-visible, +.about__table-person:focus-visible { + outline: 2px solid var(--ot-color-focus); + outline-offset: 3px; +} + +.about__table { + --mat-table-background-color: transparent; + width: 100%; + background: transparent; + + .mat-mdc-header-cell { + font-size: 0.8125rem; + font-weight: 600; + color: var(--ot-color-text-muted); + } + + .mat-mdc-cell { + color: var(--ot-color-text); + font-variant-numeric: tabular-nums; + } + + .mat-mdc-row:last-of-type .mat-mdc-cell { + border-bottom: 0; + } +} + +.about__table-person { + display: inline-flex; + align-items: center; + gap: 12px; + min-width: 0; + padding-block: 8px; + color: var(--ot-color-text); + text-decoration: none; + + &:hover .about__table-login { + color: var(--ot-color-link); + text-decoration: underline; + } +} + +.about__table-avatar { + flex: none; + width: 32px; + height: 32px; + border-radius: var(--ot-radius-circle); + background: var(--ot-color-surface-raised); +} + +.about__table-login { + overflow-wrap: anywhere; +} + +// Narrow dialogs keep the contributor and total, and drop the per-repository split, +// so the table never scrolls sideways on a phone. +@container (max-width: 559px) { + // Sub-pixel column widths can leave a 1px overflow that would show a scrollbar. + .about__table-frame { + overflow-x: clip; + } + + .about__table { + .mat-column-api-contributions, + .mat-column-web-contributions, + .mat-column-deploy-contributions, + .mat-column-io-contributions { + display: none; + } + + .mat-mdc-header-cell, + .mat-mdc-cell { + padding-inline: 12px 8px; + } + + .about__table-person { + gap: 8px; + } + } +} + +// --- Open source ---------------------------------------------------------- + +.about__open-source { + display: flex; + flex-wrap: wrap; + align-items: center; + justify-content: space-between; + gap: 12px 16px; +} + +.about__note { + flex: 1 1 260px; + margin: 0; + font-size: 0.875rem; + line-height: 1.5; + color: var(--ot-color-text); +} + +.about__note-muted { + display: block; + color: var(--ot-color-text-muted); +} + +.about__source-links { + display: flex; + flex-wrap: wrap; + gap: 8px; +} + +// --- Footer --------------------------------------------------------------- + +.about__actions.mat-mdc-dialog-actions { + gap: 8px; + min-height: 0; + padding: 8px $gutter $gutter; +} + +// --- Reduced motion ------------------------------------------------------- + +@media (prefers-reduced-motion: reduce) { + .about__card { + animation: none; + } + + .about__avatar { + transition: none; + } + + .about__card:hover .about__avatar { + transform: none; + } +} diff --git a/src/app/common/modals/date-change-modal/task-date-slider.component.html b/src/app/common/modals/date-change-modal/task-date-slider.component.html index 3471580e68..5de246b681 100644 --- a/src/app/common/modals/date-change-modal/task-date-slider.component.html +++ b/src/app/common/modals/date-change-modal/task-date-slider.component.html @@ -1,22 +1,24 @@
@if (showTaskAbbr && editMode) { - + {{ task.definition.name }} }
@if (showTaskAbbr) { - + {{ task.definition.abbreviation }}: } @else { - + } - + {{ task.localDueDateString() }} @if (task.unit.allowFlexibleDates) {
diff --git a/src/app/common/modals/date-change-modal/task-date-slider.component.spec.ts b/src/app/common/modals/date-change-modal/task-date-slider.component.spec.ts new file mode 100644 index 0000000000..73e68acf2a --- /dev/null +++ b/src/app/common/modals/date-change-modal/task-date-slider.component.spec.ts @@ -0,0 +1,66 @@ +import {beforeEach, describe, expect, it} from 'vitest'; +import {ComponentFixture, TestBed} from '@angular/core/testing'; +import {FormsModule} from '@angular/forms'; +import {MatSliderModule} from '@angular/material/slider'; +import {provideRouter} from '@angular/router'; +import {Task} from 'src/app/api/models/task'; +import {AlertService} from '../../services/alert.service'; +import {ConfirmationModalService} from '../confirmation-modal/confirmation-modal.service'; +import {TaskDateSliderComponent} from './task-date-slider.component'; + +describe('TaskDateSliderComponent labels', () => { + let fixture: ComponentFixture; + + beforeEach(async () => { + await TestBed.configureTestingModule({ + declarations: [TaskDateSliderComponent], + imports: [FormsModule, MatSliderModule], + providers: [ + provideRouter([]), + {provide: AlertService, useValue: {}}, + {provide: ConfirmationModalService, useValue: {}}, + ], + }).compileComponents(); + fixture = TestBed.createComponent(TaskDateSliderComponent); + fixture.componentRef.setInput('task', { + id: 41, + definition: {name: 'First task', abbreviation: '1.1P'}, + unit: {totalWeeks: 12, allowFlexibleDates: false}, + project: {specConDays: 0}, + dueWeek: 2, + localDueDateString: () => '20 Sep', + localDueDate: () => new Date('2026-09-20'), + localDeadlineDate: () => new Date('2026-09-30'), + } as unknown as Task); + fixture.detectChanges(); + }); + + it('associates Complete By with the native slider thumb', () => { + const label: HTMLLabelElement = fixture.nativeElement.querySelector('label'); + const input: HTMLInputElement = fixture.nativeElement.querySelector('input'); + expect(label.textContent).toContain('Complete By'); + expect(label.control).toBe(input); + expect(input.disabled).toBe(true); + expect(input.id).toBe('task-due-date-41'); + }); + + it('keeps the thumb named when the task abbreviation replaces the label', async () => { + fixture.componentRef.setInput('showTaskAbbr', true); + fixture.detectChanges(); + fixture.componentInstance.editMode = true; + fixture.detectChanges(); + await fixture.whenStable(); + const input: HTMLInputElement = fixture.nativeElement.querySelector('input'); + expect(input.getAttribute('aria-label')).toBe('Complete by for 1.1P'); + expect(input.disabled).toBe(false); + expect(fixture.nativeElement.querySelector('label')).toBeNull(); + }); + + it('renders deadline warnings as text rather than control labels', () => { + fixture.componentInstance.editMode = true; + fixture.componentInstance.task.localDeadlineDate = () => new Date('2026-09-19'); + fixture.detectChanges(); + expect(fixture.nativeElement.querySelector('p')?.textContent).toContain('Warning:'); + expect(fixture.nativeElement.querySelectorAll('label')).toHaveLength(1); + }); +}); diff --git a/src/app/common/modals/scorm-extension-modal/scorm-extension-modal.component.html b/src/app/common/modals/scorm-extension-modal/scorm-extension-modal.component.html index 89b4341889..c429a7c851 100644 --- a/src/app/common/modals/scorm-extension-modal/scorm-extension-modal.component.html +++ b/src/app/common/modals/scorm-extension-modal/scorm-extension-modal.component.html @@ -30,7 +30,7 @@

Extra attempt request

- +

- + '; + const result = await expectAccessible(fixture); + expect(result.violations).toEqual([]); + expect(result.passes.some((rule) => rule.id === 'button-name')).toBe(true); + }); + + it('fails the check for a deliberately unnamed button', async () => { + fixture.innerHTML = ''; + await expect(expectAccessible(fixture)).rejects.toThrow( + /button-name \((serious|critical)\)[\s\S]*unnamed-action/, + ); + }); + + it('does not keep a failed fixture in a baseline after the defect is fixed', async () => { + fixture.innerHTML = ''; + await expect(expectAccessible(fixture)).rejects.toThrow('button-name'); + fixture.querySelector('button').setAttribute('aria-label', 'Save changes'); + await expect(expectAccessible(fixture)).resolves.toMatchObject({violations: []}); + }); + + it('keeps native label association checks enabled', async () => { + fixture.innerHTML = ''; + await expect(expectAccessible(fixture)).rejects.toThrow(/label \((serious|critical)\)/); + fixture.insertAdjacentHTML('afterbegin', ''); + await expect(expectAccessible(fixture)).resolves.toMatchObject({violations: []}); + }); + + it('rejects detached fixtures instead of reporting an empty scan as a pass', async () => { + fixture.remove(); + await expect(expectAccessible(fixture)).rejects.toThrow('attached to the document'); + }); +}); diff --git a/src/app/common/testing/accessibility.ts b/src/app/common/testing/accessibility.ts new file mode 100644 index 0000000000..d2f9b00dbb --- /dev/null +++ b/src/app/common/testing/accessibility.ts @@ -0,0 +1,36 @@ +import axe, {AxeResults} from 'axe-core'; + +/** + * Check a rendered, attached fixture after Angular's fixture.whenStable(). + * Await this helper with real timers; axe runs asynchronously. + * + * This is a DOM regression check, not a complete WCAG audit. jsdom cannot + * measure layout or colour contrast, so color-contrast is the only disabled + * rule. Keep contrast-math tests and manual browser checks alongside it. + * https://github.com/dequelabs/axe-core#supported-browsers + * + * Incomplete results need manual review and remain available in the return + * value. No violation baseline or severity filter hides newly detected issues. + */ +export async function expectAccessible(element: HTMLElement): Promise { + if (!element?.isConnected) { + throw new Error('Accessibility checks require a rendered element attached to the document.'); + } + + const results = await axe.run(element, { + // Inspect local fixture styles only; never fetch external stylesheets/media. + preload: false, + rules: {'color-contrast': {enabled: false}}, + }); + + if (results.violations.length > 0) { + const failures = results.violations.map((violation) => { + const targets = violation.nodes.map((node) => JSON.stringify(node.target)).join(', '); + return `${violation.id} (${violation.impact}): ${violation.help}\n Targets: ${targets}`; + }); + // Report selectors and rule names without logging fixture HTML or user data. + throw new Error(`Accessibility violations:\n${failures.join('\n')}`); + } + + return results; +} diff --git a/src/app/eula/accept-eula/accept-eula.component.html b/src/app/eula/accept-eula/accept-eula.component.html index 5de43cf604..39e58a2e47 100644 --- a/src/app/eula/accept-eula/accept-eula.component.html +++ b/src/app/eula/accept-eula/accept-eula.component.html @@ -8,6 +8,7 @@

End User License Agreements

- } -
+ + } -
+
@if (linkedUnit) { - {{ linkedUnit.code }} — {{ linkedUnit.name }} - @if (currentUser?.systemRole === 'Convenor' || currentUser?.systemRole === 'Admin') { -
- +

{{ linkedUnit.code }} — {{ linkedUnit.name }}

+ @if (canManageLink) { + } } @else { - This course has not yet been linked to an OnTrack Unit. - - @if (currentUser?.systemRole === 'Convenor' || currentUser?.systemRole === 'Admin') { -
- +

This course has not been linked to an OnTrack unit yet.

+ @if (canManageLink) { + } @else { -
- Only OnTrack staff are permitted to link this course. +

Only OnTrack staff can link this course.

} }
diff --git a/src/app/projects/states/dashboard/dashboard.tpl.html b/src/app/projects/states/dashboard/dashboard.tpl.html deleted file mode 100644 index 2171877d44..0000000000 --- a/src/app/projects/states/dashboard/dashboard.tpl.html +++ /dev/null @@ -1,38 +0,0 @@ -
- - - - - - - - -
diff --git a/src/app/projects/states/dashboard/directives/student-task-list/student-task-list.tpl.html b/src/app/projects/states/dashboard/directives/student-task-list/student-task-list.tpl.html deleted file mode 100644 index 5e666e5409..0000000000 --- a/src/app/projects/states/dashboard/directives/student-task-list/student-task-list.tpl.html +++ /dev/null @@ -1,98 +0,0 @@ -
-
- -
- -
diff --git a/src/app/projects/states/dashboard/directives/task-dashboard/directives/task-assessment-card/task-assessment-card.tpl.html b/src/app/projects/states/dashboard/directives/task-dashboard/directives/task-assessment-card/task-assessment-card.tpl.html deleted file mode 100644 index 0aa6c58092..0000000000 --- a/src/app/projects/states/dashboard/directives/task-dashboard/directives/task-assessment-card/task-assessment-card.tpl.html +++ /dev/null @@ -1,67 +0,0 @@ -
-
-

Assessment Information

-
-
-
-
This task {{assessmentCards.hasBeenGraded ? 'has been' : 'will be'}} assigned a grade
-
-

- This task will be graded against a grade standard. Your work will be assessed and assigned - a grade according to a Pass, Credit, Distinction or High Distinction standard. -

-
-
Advice for achieving a {{task.project.targetGradeWord}}
-

- As you are attempting to achieve a {{task.project.targetGradeWord}} in this unit, you - should attempt to achieve a {{task.project.targetGradeWord}} grade on - this task. Ask your tutor to find out more on what they are looking for when they are - assessing this work to a specific grade. -

-
-
- -
- Your tutor has marked you on this task to a {{task.gradeWord}} standard. -
- -
- -
-
-
- - This task will be assessed on a scale to {{task.definition.maxQualityPts}} - - This task has been assessed for quality -
-

- This task will be graded against a quality scale from - 0 to {{task.definition.maxQualityPts}}. Your work will assessed and - assigned a star rating based on the quality of your submission. -

-
- - -

- You have been awarded out - {{task.qualityPts}} of {{task.definition.maxQualityPts}} - avaliable points for this task. -

-
-
- -
- -
- diff --git a/src/app/projects/states/dashboard/directives/task-dashboard/directives/task-due-card/task-due-card.tpl.html b/src/app/projects/states/dashboard/directives/task-dashboard/directives/task-due-card/task-due-card.tpl.html deleted file mode 100644 index a4ea4b1999..0000000000 --- a/src/app/projects/states/dashboard/directives/task-dashboard/directives/task-due-card/task-due-card.tpl.html +++ /dev/null @@ -1,109 +0,0 @@ -
- -
- -
-
-

Aim To Complete Soon - Due in {{task.timeUntilDueDateDescription()}}

-
-
- -

- This task's due date is {{task.localDueDateString()}}. You should aim to - complete this task before then to keep your progress on track. -

-
-
- -

- This task's due date is {{task.localDueDateString()}}. Make sure to - discuss this task with your tutor as soon as possible. -

-

- Tasks are only considered Completed once your tutor has - Discussed your work with you. -

-
-
-
- -
-
-
-

Past Due Date By {{task.timePastDueDateDescription()}}

-
-
- -

- You should have completed this task by {{task.localDueDateString()}}. Try - and finish it as soon as possible to avoid falling behind. As you will submit this task - after the deadline for feedback, it will not be reviewed by a tutor and it is now your - sole responsibility to ensure that this submission meets the required standard. The task - will be assessed as part of the portfolio. -

-

- Aim to submit future tasks before the deadline to make good use of the opportunity to - receive feedback on your work and your tutor will work with you to make sure that your - submission meets all the requirements. -

-
-
- -

- You should have completed this task by {{task.localDueDateString()}}. - Make sure to discuss this task with your tutor as soon as possible. If this task remains - on this state for an extended period, it will be marked as Time Exceeded. -

-

- Tasks are only considered completed once your tutor has - discussed your work with you. -

-
-
-
- -
-
-
-

Passed Due Date By {{task.timePastDueDateDescription()}}

-
-
- -

- You should have completed this task by {{task.localDueDateString()}}. - This task is now past the deadline and will be marked as Time Exceeded when - submitted. You should consult with the unit assessment details to determine the impact of - failing to complete this task within the allocated time. -

-
-
- -

- You should have completed this task by {{task.localDueDateString()}}. - Make sure to discuss this task with your tutor as soon as possible. -

-

- Tasks are only considered Completed once it demonstrates the required - standard, and it is discussed with your tutor. -

-
-
-
-
- -
-
-

Wait for Tutor Feedback

-
-
-

- You have submitted this task and should now wait for feedback from your tutor. - Do not re-upload new files at this time as the status will be changed to - Time Exceeded. -

-
-
diff --git a/src/app/projects/states/dashboard/directives/task-dashboard/directives/task-ilos-card/task-ilos-card.accessibility.spec.ts b/src/app/projects/states/dashboard/directives/task-dashboard/directives/task-ilos-card/task-ilos-card.accessibility.spec.ts new file mode 100644 index 0000000000..8279811092 --- /dev/null +++ b/src/app/projects/states/dashboard/directives/task-dashboard/directives/task-ilos-card/task-ilos-card.accessibility.spec.ts @@ -0,0 +1,74 @@ +import {beforeEach, describe, expect, it} from 'vitest'; +import {TestBed} from '@angular/core/testing'; +import {MatCardModule} from '@angular/material/card'; +import {MatChipsModule} from '@angular/material/chips'; +import {MatTooltipModule} from '@angular/material/tooltip'; +import {LearningOutcome} from 'src/app/api/models/learning-outcome'; +import {expectAccessible} from 'src/app/common/testing/accessibility'; +import {TaskIlosCardComponent} from './task-ilos-card.component'; + +describe('Task learning outcomes rendered accessibility', () => { + beforeEach(async () => { + await TestBed.configureTestingModule({ + declarations: [TaskIlosCardComponent], + imports: [MatCardModule, MatChipsModule, MatTooltipModule], + }).compileComponents(); + }); + + it('presents linked outcomes as descriptive list items without grid roles or tab stops', async () => { + const fixture = TestBed.createComponent(TaskIlosCardComponent); + const outcomes = [ + { + id: 1, + abbreviation: 'LO1', + fullOutcomeDescription: 'Explain the solution.', + linkedOutcomeIds: [2, 3], + }, + { + id: 2, + abbreviation: 'O1', + shortDescription: 'Critical thinking', + fullOutcomeDescription: 'Think critically.', + linkedOutcomeIds: [], + }, + { + id: 3, + abbreviation: 'O2', + shortDescription: 'Clear communication', + fullOutcomeDescription: 'Communicate clearly.', + linkedOutcomeIds: [], + }, + ] as LearningOutcome[]; + fixture.componentRef.setInput('iloContextType', 'Unit'); + fixture.componentRef.setInput('unit', {ilos: outcomes}); + fixture.detectChanges(); + await fixture.whenStable(); + + const root: HTMLElement = fixture.nativeElement; + await expectAccessible(root); + const lists = root.querySelectorAll('[role="list"]'); + expect(lists).toHaveLength(1); + expect(lists[0].getAttribute('aria-label')).toBe('Linked outcomes for LO1'); + const chips = Array.from(lists[0].querySelectorAll('[role="listitem"]')); + expect(chips.map((chip) => chip.textContent.trim())).toEqual(['O1', 'O2']); + expect(root.querySelector('mat-chip-row, [role="gridcell"]')).toBeNull(); + for (const chip of chips) { + expect(chip.tabIndex).toBe(-1); + expect(chip.querySelector('button, input, [role="option"], [tabindex="0"]')).toBeNull(); + } + expect( + chips.map( + (chip) => document.getElementById(chip.getAttribute('aria-describedby'))?.textContent, + ), + ).toEqual(['Critical thinking', 'Clear communication']); + }); + + it('does not render an empty outcomes card or an empty linked-outcome list', async () => { + const fixture = TestBed.createComponent(TaskIlosCardComponent); + fixture.componentRef.setInput('iloContextType', 'Unit'); + fixture.componentRef.setInput('unit', {ilos: []}); + fixture.detectChanges(); + await fixture.whenStable(); + expect(fixture.nativeElement.querySelector('mat-card, [role="list"]')).toBeNull(); + }); +}); diff --git a/src/app/projects/states/dashboard/directives/task-dashboard/directives/task-status-card/task-status-card.component.html b/src/app/projects/states/dashboard/directives/task-dashboard/directives/task-status-card/task-status-card.component.html index 0594581649..3750f43643 100644 --- a/src/app/projects/states/dashboard/directives/task-dashboard/directives/task-status-card/task-status-card.component.html +++ b/src/app/projects/states/dashboard/directives/task-dashboard/directives/task-status-card/task-status-card.component.html @@ -1,6 +1,12 @@ - + @if (triggers?.length > 0) { + Task status -

{{ task?.statusLabel() }}

+ {{ task?.statusLabel() }} @for (trigger of triggers; track trigger) {
- -
{{ trigger.label }}
+ + + {{ trigger.label }}
} @@ -33,7 +46,7 @@
{{ trigger.label }}
style="margin-right: 10px" [status]="$safeNavigationMigration(task?.status)" > -
{{ task?.statusLabel() }}
+ {{ task?.statusLabel() }} } @@ -47,36 +60,42 @@
{{ task?.statusLabel() }}
} - -
- + +
+ @if (showUploadSubmission) { + + } @if (task?.canApplyForExtension()) { - + } - @if (task?.inSubmittedState() && task?.requiresFileUpload()) { - + @if (showUploadNewFiles) { + }
-
- -
- - - -
+ + + + diff --git a/src/app/projects/states/dashboard/directives/task-dashboard/directives/task-status-card/task-status-card.component.spec.ts b/src/app/projects/states/dashboard/directives/task-dashboard/directives/task-status-card/task-status-card.component.spec.ts index d377730a00..891dd57d79 100644 --- a/src/app/projects/states/dashboard/directives/task-dashboard/directives/task-status-card/task-status-card.component.spec.ts +++ b/src/app/projects/states/dashboard/directives/task-dashboard/directives/task-status-card/task-status-card.component.spec.ts @@ -1,8 +1,18 @@ -import {beforeEach, describe, expect, it} from 'vitest'; +import {beforeEach, describe, expect, it, vi} from 'vitest'; +import {OverlayContainer} from '@angular/cdk/overlay'; import {NO_ERRORS_SCHEMA} from '@angular/core'; import {ComponentFixture, TestBed} from '@angular/core/testing'; +import {MatButtonModule} from '@angular/material/button'; +import {MatCardModule} from '@angular/material/card'; +import {MatFormFieldModule} from '@angular/material/form-field'; +import {MatMenuModule} from '@angular/material/menu'; +import {MatSelectModule} from '@angular/material/select'; import {ActivatedRoute} from '@angular/router'; import {EMPTY} from 'rxjs'; +import {Task} from 'src/app/api/models/task'; +import {TaskDefinition} from 'src/app/api/models/task-definition'; +import {TaskStatus} from 'src/app/api/models/task-status'; +import {TaskStatusEnum} from 'src/app/api/models/task-status'; import {TaskService} from 'src/app/api/services/task.service'; import {UserService} from 'src/app/api/services/user.service'; import {ExtensionModalService} from 'src/app/common/modals/extension-modal/extension-modal.service'; @@ -24,6 +34,7 @@ describe('TaskStatusCardComponent', () => { beforeEach(async () => { await TestBed.configureTestingModule({ declarations: [TaskStatusCardComponent], + imports: [MatButtonModule, MatCardModule, MatFormFieldModule, MatMenuModule, MatSelectModule], providers: [ {provide: ExtensionModalService, useValue: emptyProvider}, {provide: TaskService, useValue: taskServiceStub}, @@ -31,21 +42,150 @@ describe('TaskStatusCardComponent', () => { {provide: QrModalService, useValue: emptyProvider}, {provide: DoubtfireConstants, useValue: emptyProvider}, {provide: SubmissionTypeModalService, useValue: emptyProvider}, - {provide: UserService, useValue: emptyProvider}, + {provide: UserService, useValue: {currentUser: {systemRole: 'Student'}}}, {provide: FeedbackAppealModalService, useValue: emptyProvider}, ], schemas: [NO_ERRORS_SCHEMA], - }) - .overrideComponent(TaskStatusCardComponent, {set: {template: ''}}) - .compileComponents(); + }).compileComponents(); }); beforeEach(() => { fixture = TestBed.createComponent(TaskStatusCardComponent); component = fixture.componentInstance; + component.task = { + status: 'working_on_it', + statusLabel: () => 'Working On It', + statusHelp: () => ({reason: 'Keep working.', action: ''}), + blockedByPrerequisiteTasks: vi.fn().mockReturnValue(false), + canApplyForExtension: () => false, + inSubmittedState: () => false, + hasSubmissionHistory: () => false, + requiresFileUpload: () => true, + triggerTransition: vi.fn(), + } as unknown as Task; + component.triggers = [ + TaskStatus.statusData('working_on_it'), + TaskStatus.statusData('need_help'), + ]; + fixture.detectChanges(); }); - it('should create', () => { - expect(component).toBeTruthy(); + const combobox = (): HTMLElement => fixture.nativeElement.querySelector('[role="combobox"]'); + + const accessibleLabel = (): string => + combobox() + .getAttribute('aria-labelledby') + .split(' ') + .map((id) => document.getElementById(id)?.textContent) + .join(' '); + + it('names the status combobox and keeps status text out of the heading list', async () => { + expect(accessibleLabel()).toContain('Task status'); + expect(fixture.nativeElement.querySelector('h2, h5')).toBeNull(); + + combobox().click(); + fixture.detectChanges(); + await fixture.whenStable(); + + const overlay = TestBed.inject(OverlayContainer).getContainerElement(); + expect(overlay.querySelector('h2, h5')).toBeNull(); + const options = Array.from(overlay.querySelectorAll('[role="option"]')); + expect(options).toHaveLength(2); + expect(options[1].textContent).toContain('Need Help'); + options[1].click(); + fixture.detectChanges(); + + expect(component.task.triggerTransition).toHaveBeenCalledWith('need_help'); + }); + + it('keeps the label available when prerequisites disable the selector', () => { + vi.mocked(component.task.blockedByPrerequisiteTasks).mockReturnValue(true); + fixture.detectChanges(); + + expect(accessibleLabel()).toContain('Task status'); + expect(combobox().getAttribute('aria-disabled')).toBe('true'); + combobox().click(); + fixture.detectChanges(); + expect( + TestBed.inject(OverlayContainer).getContainerElement().querySelector('[role="listbox"]'), + ).toBeNull(); + expect(component.task.triggerTransition).not.toHaveBeenCalled(); + }); + + function buildTask(status: TaskStatusEnum, requiresFiles: boolean, submissionDate?: Date): Task { + const task = new Task(); + task.status = status; + task.definition = { + uploadRequirements: requiresFiles ? [{key: 'file0'}] : [], + } as unknown as TaskDefinition; + task.submissionDate = submissionDate; + return task; + } + + const previousSubmission = new Date('2026-08-31T00:00:00Z'); + + it.each([ + {status: 'not_started', requiresFiles: true, submitted: undefined, first: true, again: false}, + {status: 'not_started', requiresFiles: false, submitted: undefined, first: true, again: false}, + { + status: 'fix_and_resubmit', + requiresFiles: true, + submitted: previousSubmission, + first: true, + again: false, + }, + {status: 'redo', requiresFiles: true, submitted: previousSubmission, first: true, again: false}, + {status: 'redo', requiresFiles: false, submitted: undefined, first: true, again: false}, + { + status: 'working_on_it', + requiresFiles: true, + submitted: previousSubmission, + first: true, + again: false, + }, + { + status: 'ready_for_feedback', + requiresFiles: true, + submitted: previousSubmission, + first: false, + again: true, + }, + { + status: 'complete', + requiresFiles: true, + submitted: previousSubmission, + first: false, + again: true, + }, + {status: 'complete', requiresFiles: false, submitted: undefined, first: false, again: false}, + ] as const)( + 'shows exactly one appropriate upload action for $status (uploads: $requiresFiles)', + ({status, requiresFiles, submitted, first, again}) => { + component.task = buildTask(status, requiresFiles, submitted); + + expect(component.showUploadSubmission).toBe(first); + expect(component.showUploadNewFiles).toBe(again); + }, + ); + + it('offers the full submission flow again for a task returned for resubmission', () => { + const task = buildTask('fix_and_resubmit', true, previousSubmission); + task.definition.assessInPortfolioOnly = false; + const triggerTransition = vi.spyOn(task, 'triggerTransition').mockResolvedValue(); + component.task = task; + + expect(component.showUploadSubmission).toBe(true); + expect(component.showUploadNewFiles).toBe(false); + component.uploadSubmission(); + + expect(triggerTransition).toHaveBeenCalledWith('ready_for_feedback'); + }); + + it('marks the replacement action pending while details or PDF processing is unresolved', () => { + component.task = {processingPdf: true, loadingSubmissionDetails: false} as Task; + expect(component.submissionActionPending).toBe(true); + + component.task = {processingPdf: false, loadingSubmissionDetails: true} as Task; + expect(component.submissionActionPending).toBe(true); }); }); diff --git a/src/app/projects/states/dashboard/directives/task-dashboard/task-dashboard.accessibility.spec.ts b/src/app/projects/states/dashboard/directives/task-dashboard/task-dashboard.accessibility.spec.ts new file mode 100644 index 0000000000..6178a95d2e --- /dev/null +++ b/src/app/projects/states/dashboard/directives/task-dashboard/task-dashboard.accessibility.spec.ts @@ -0,0 +1,97 @@ +import {beforeEach, describe, expect, it} from 'vitest'; +import {NO_ERRORS_SCHEMA} from '@angular/core'; +import {ComponentFixture, TestBed} from '@angular/core/testing'; +import {MatButtonModule} from '@angular/material/button'; +import {MatIconModule} from '@angular/material/icon'; +import {MatMenuModule} from '@angular/material/menu'; +import {MatProgressSpinnerModule} from '@angular/material/progress-spinner'; +import {MatTabsModule} from '@angular/material/tabs'; +import {ActivatedRoute} from '@angular/router'; +import {BehaviorSubject, Subject} from 'rxjs'; +import {Task} from 'src/app/api/models/task'; +import {TaskService} from 'src/app/api/services/task.service'; +import {UserService} from 'src/app/api/services/user.service'; +import {FileDownloaderService} from 'src/app/common/file-downloader/file-downloader.service'; +import {DashboardViews, SelectedTaskService} from '../../selected-task.service'; +import {TaskDashboardComponent} from './task-dashboard.component'; + +describe('Task dashboard rendered tabs', () => { + let fixture: ComponentFixture; + + beforeEach(async () => { + await TestBed.configureTestingModule({ + declarations: [TaskDashboardComponent], + imports: [ + MatButtonModule, + MatIconModule, + MatMenuModule, + MatProgressSpinnerModule, + MatTabsModule, + ], + providers: [ + { + provide: TaskService, + useValue: { + markedStatuses: [], + statusSeq: new Map(), + taskSubmissionCompleted$: new Subject(), + }, + }, + {provide: UserService, useValue: {}}, + {provide: ActivatedRoute, useValue: {}}, + {provide: FileDownloaderService, useValue: {}}, + { + provide: SelectedTaskService, + useValue: {currentView$: new BehaviorSubject(DashboardViews.details)}, + }, + ], + schemas: [NO_ERRORS_SCHEMA], + }).compileComponents(); + fixture = TestBed.createComponent(TaskDashboardComponent); + fixture.componentInstance.task = { + definition: {hasTaskSheet: true}, + hasPdf: false, + processingPdf: false, + project: {}, + unit: {staff: []}, + submissionUrl: () => '/synthetic-submission.pdf', + hasSubmissionHistory() { + return this.hasPdf || this.processingPdf; + }, + } as unknown as Task; + fixture.detectChanges(); + await fixture.whenStable(); + }); + + it('retains Material disabled semantics for an unavailable submission', () => { + const root: HTMLElement = fixture.nativeElement; + const tabs = Array.from(root.querySelectorAll('[role="tab"]')); + expect(tabs.map((tab) => tab.textContent.trim())).toEqual([ + 'Task Details', + 'Task Sheet', + 'Your Submission', + ]); + expect(tabs[0].getAttribute('aria-selected')).toBe('true'); + expect(tabs[1].getAttribute('aria-disabled')).toBe('false'); + expect(tabs[2].getAttribute('aria-disabled')).toBe('true'); + expect(tabs[2].classList.contains('mat-mdc-tab-disabled')).toBe(true); + expect(tabs[2].tabIndex).toBe(-1); + tabs[2].click(); + expect(fixture.componentInstance.currentView).toBe(DashboardViews.details); + }); + + it('keeps an available submission as an enabled, selectable inactive tab', async () => { + Object.assign(fixture.componentInstance.task, {hasPdf: true}); + fixture.detectChanges(); + await fixture.whenStable(); + const submission: HTMLElement = fixture.nativeElement.querySelectorAll('[role="tab"]')[2]; + expect(submission.getAttribute('aria-disabled')).toBe('false'); + expect(submission.classList.contains('mat-mdc-tab-disabled')).toBe(false); + expect(submission.getAttribute('aria-selected')).toBe('false'); + submission.click(); + fixture.detectChanges(); + await fixture.whenStable(); + expect(fixture.componentInstance.currentView).toBe(DashboardViews.submission); + expect(submission.getAttribute('aria-selected')).toBe('true'); + }); +}); diff --git a/src/app/projects/states/dashboard/directives/task-dashboard/task-dashboard.component.scss b/src/app/projects/states/dashboard/directives/task-dashboard/task-dashboard.component.scss index 2b2f7b6954..dafd0f62c3 100644 --- a/src/app/projects/states/dashboard/directives/task-dashboard/task-dashboard.component.scss +++ b/src/app/projects/states/dashboard/directives/task-dashboard/task-dashboard.component.scss @@ -1,18 +1,88 @@ +@use '@angular/material' as mat; + .empty-state-icon { height: 120px; width: 120px; font-size: 120px; - color: #c5c5c5; + color: var(--ot-color-text-muted); } .no-selection-text { font-size: 2.5rem; - color: #c5c5c5; + color: var(--ot-color-text-muted); +} + +.submission-view { + display: grid; + gap: 0.75rem; + grid-template-rows: auto minmax(20rem, 1fr); + height: 100%; + min-height: 0; + padding: 0.75rem; +} + +.submission-preview { + min-height: 20rem; + min-width: 0; } +// On a wider screen the submission view scrolls as one: the summary card, then the +// PDF as tall as the panel. Scrolling past the card gives the document the whole +// panel instead of the strip the card left it. (Phones size it to the screen below.) +@media (min-width: 640px) { + .task-dashboard-body { + container-type: size; + } + + .submission-view { + grid-template-rows: auto auto; + height: auto; + } + + .submission-preview { + height: calc(100cqh - 1.5rem); + } +} + +.submission-preview-placeholder { + align-items: center; + border: 1px dashed var(--ot-color-border); + border-radius: 0.75rem; + color: var(--ot-color-text-muted); + display: flex; + flex-direction: column; + gap: 0.5rem; + justify-content: center; + min-height: 16rem; + padding: 1.5rem; + text-align: center; +} + +.submission-preview-placeholder mat-icon { + font-size: 3rem; + height: 3rem; + width: 3rem; +} + +// Keep the tab strip aligned to its start so narrow screens can reach every label. :host ::ng-deep .task-dashboard-tabs { .mat-mdc-tab-header { - justify-content: center; + // Filled-control primary colours are not text colours on every theme surface. + @include mat.tabs-overrides( + ( + active-label-text-color: var(--ot-color-link), + active-focus-label-text-color: var(--ot-color-link), + active-hover-label-text-color: var(--ot-color-link), + active-indicator-color: var(--ot-color-link), + active-focus-indicator-color: var(--ot-color-link), + active-hover-indicator-color: var(--ot-color-link), + inactive-label-text-color: var(--ot-color-text-muted), + inactive-focus-label-text-color: var(--ot-color-text-muted), + inactive-hover-label-text-color: var(--ot-color-text-muted), + ) + ); + + justify-content: flex-start; max-width: 100%; overflow-x: auto; overflow-y: hidden; @@ -21,7 +91,7 @@ .mat-mdc-tab-label-container { flex: 0 0 auto; - margin: 0 auto; + margin: 0; overflow: visible; } @@ -30,3 +100,55 @@ width: max-content; } } + +// A card with nothing to show leaves an empty host, or a host around a hidden card. +// Either one still counts as a flex item, so it added a second gap and the space +// between cards jumped about. ::ng-deep because the hidden card is in the child's view. +:host ::ng-deep .task-details > :empty, +:host ::ng-deep .task-details > :has(> [hidden]:only-child) { + display: none; +} + +@media (max-width: 639.98px) { + // Details is a normal document on phones. PDF, submission, history and + // moderation views keep their purpose-built bounded viewers. + .task-dashboard-shell--document { + height: auto !important; + min-height: 0; + overflow: visible; + } + + .task-dashboard-body--document { + min-height: 0; + flex: none; + overflow: visible; + } + + .submission-view { + grid-template-rows: auto minmax(18rem, 1fr); + padding: 0.5rem; + } + + // The page scrolls as one document on a phone, so a PDF sized to "100%" of it fell + // back to its minimum height and left blank page below. It takes the screen under + // the sticky task tabs instead, and once the headings above it have scrolled away + // it runs from the tabs to the bottom edge, through the page's bottom padding. + .task-dashboard-body > f-pdf-viewer, + .submission-preview { + display: block; + height: calc(100dvh - var(--mobile-task-tabs-height)); + margin-bottom: calc(-1 * var(--mobile-page-bottom-padding)); + } + + .submission-view { + padding-bottom: 0; + } +} + +// Full screen, the reading tabs (notes, history, details) keep a comfortable line length +// in the middle of the page. The PDF viewers keep the whole width. +.dashboard-reading-measure { + width: 100%; + max-width: 960px; + margin-inline: auto; +} diff --git a/src/app/projects/states/dashboard/directives/task-dashboard/task-dashboard.tpl.html b/src/app/projects/states/dashboard/directives/task-dashboard/task-dashboard.tpl.html deleted file mode 100644 index b7adf8c446..0000000000 --- a/src/app/projects/states/dashboard/directives/task-dashboard/task-dashboard.tpl.html +++ /dev/null @@ -1,130 +0,0 @@ -
-
-
- {{task.definition.name}} - {{task.definition.name}} - -
- - -
- -
-
- -
- - - - - - - - - - - - - - - - -
- - -
- - Warning: This task has {{task.definition.taskPrerequisitesCache.currentValues.length}} - prerequisite{{ task.definition.taskPrerequisitesCache.currentValues.length > 1 ? 's' : '' }} - that you still need to complete. You won’t be able to submit this task until all prerequisites - are met. -
- -
- -
- -
- - -
-
- -
-
- -
- -
- - PDF Still Processing - No Submission Uploaded - No Task Selected -
- -
-
- -
- -
- -
diff --git a/src/app/projects/states/jplag/jplag-report-viewer.component.html b/src/app/projects/states/jplag/jplag-report-viewer.component.html index 1b5e99f233..df7342d9c1 100644 --- a/src/app/projects/states/jplag/jplag-report-viewer.component.html +++ b/src/app/projects/states/jplag/jplag-report-viewer.component.html @@ -4,6 +4,7 @@ frameborder="0" src="/JPlag/" style="overflow: hidden" + title="JPlag similarity report" [hidden]="hidden" [scrolling]="false" > diff --git a/src/app/projects/states/plan/task-planner/task-planner.component.scss b/src/app/projects/states/plan/task-planner/task-planner.component.scss index df94c4a868..681bea14b8 100644 --- a/src/app/projects/states/plan/task-planner/task-planner.component.scss +++ b/src/app/projects/states/plan/task-planner/task-planner.component.scss @@ -1,3 +1,16 @@ +:host { + display: block; + color: var(--ot-color-text); +} + +.planner-empty { + margin: 0; + padding: 16px; + color: var(--ot-color-text-muted); + font-size: 0.9rem; + line-height: 1.5; +} + :host ::ng-deep .flexible-dates .gantt-links-overlay svg { z-index: 999 !important; pointer-events: none; @@ -17,6 +30,9 @@ } :host ::ng-deep gantt-calendar-header .today-rect { + // The chip is filled with the warning colour (styles.scss), so its date takes the + // matching on-colour rather than the library's white. + color: var(--ot-color-on-warning) !important; display: flex; height: 28px !important; transform: translateY(-0.35rem); @@ -36,6 +52,233 @@ background-color: var(--bar-bg); } +// The bar is reachable by tab, so its focus has to be visible. currentColor is already +// chosen per bar to contrast with that bar's background, so it works on every colour the +// planner uses. +.gantt-bar:focus-visible { + outline: 2px solid currentColor; + outline-offset: -3px; +} + .flash { transition: background-color 500ms ease-in-out; } + +// Legend: a swatch per bar colour, and the today line drawn the way the chart +// draws it, so each key reads as the mark it explains. +.planner-legend__item { + display: inline-flex; + align-items: center; + gap: 0.375rem; +} + +// Each swatch keeps the bar's fill and gets an edge in the matching graphic colour +// (set per swatch in the template), so it holds 3:1 against the dark card too. +.planner-legend__swatch { + width: 1rem; + height: 0.625rem; + flex: none; + border: 1px solid; + border-radius: var(--ot-radius-xs); +} + +.planner-legend__today { + width: 2px; + height: 0.875rem; + flex: none; + background: var(--ot-color-warning); +} + +// Inset, because the gantt clips its table cells and an outward ring would be cut off. +.planner-task-link:focus-visible { + outline: 2px solid var(--ot-color-focus); + outline-offset: -3px; +} + +.planner-toolbar { + display: flex; + flex-wrap: wrap; + align-items: center; + justify-content: space-between; + gap: 1rem; +} + +.planner-toggles, +.planner-date-actions { + display: flex; + flex-wrap: wrap; + align-items: center; + gap: 0.75rem; +} + +.mobile-planner { + display: none; +} + +@media (max-width: 639.98px) { + :host { + display: block; + min-width: 0; + overflow-x: clip; + } + + .planner-toolbar, + .planner-toggles, + .planner-date-actions { + align-items: stretch; + flex-direction: column; + } + + .planner-toolbar { + gap: 0.875rem; + padding: 0.75rem 0 !important; + } + + .planner-toggles { + gap: 0.5rem; + } + + .planner-toggles mat-slide-toggle, + .planner-date-actions button { + min-height: 44px; + } + + .planner-legend, + .desktop-planner { + display: none !important; + } + + .mobile-planner { + display: grid; + gap: 0.75rem; + padding-bottom: var(--ontrack-safe-bottom-space, 1rem); + } + + .mobile-planner-empty { + padding: 2rem 1rem; + border: 1px solid var(--ot-color-border); + border-radius: 0.75rem; + color: var(--ot-color-text-muted); + text-align: center; + } + + .mobile-task-card { + min-width: 0; + padding: 1rem; + border: 1px solid var(--ot-color-border); + border-radius: 0.75rem; + background: var(--ot-color-surface); + box-shadow: 0 1px 2px rgb(0 0 0 / 8%); + } + + .mobile-task-card__header { + display: grid; + grid-template-columns: minmax(0, 1fr) auto; + align-items: start; + gap: 0.75rem; + } + + .mobile-task-card__header h3 { + margin: 0; + overflow-wrap: anywhere; + font-size: 1rem; + font-weight: 650; + line-height: 1.35; + } + + .mobile-task-card__grade { + margin: 0 0 0.25rem; + color: var(--ot-color-text-muted); + font-size: 0.8125rem; + font-weight: 600; + } + + .mobile-task-card__status { + display: flex; + max-width: 8.5rem; + align-items: center; + justify-content: flex-end; + gap: 0.25rem; + color: var(--ot-color-text); + font-size: 0.75rem; + line-height: 1.2; + text-align: right; + } + + .mobile-task-card__date-summary { + margin: 0.875rem 0 0; + color: var(--ot-color-text); + font-size: 0.875rem; + } + + .mobile-task-card__dates { + display: grid; + grid-template-columns: repeat(3, minmax(0, 1fr)); + gap: 0.5rem; + margin: 0.75rem 0 0; + } + + .mobile-task-card__dates > div { + min-width: 0; + padding: 0.625rem; + border-radius: 0.5rem; + background: var(--ot-color-surface-raised); + } + + .mobile-task-card__dates dt { + color: var(--ot-color-text-muted); + font-size: 0.6875rem; + font-weight: 600; + line-height: 1.2; + } + + .mobile-task-card__dates dd { + margin: 0.25rem 0 0; + color: var(--ot-color-text); + font-size: 0.8125rem; + line-height: 1.25; + } + + .mobile-task-card__connections { + display: flex; + align-items: center; + gap: 0.5rem; + margin-top: 0.75rem; + color: var(--ot-color-text); + font-size: 0.8125rem; + } + + .mobile-task-card__connections mat-icon { + width: 1.25rem; + height: 1.25rem; + flex: 0 0 1.25rem; + font-size: 1.25rem; + } + + .mobile-task-card__actions { + display: grid; + grid-template-columns: minmax(0, 1fr) auto; + align-items: center; + gap: 0.5rem; + margin-top: 0.875rem; + } + + .mobile-task-card__actions button, + .mobile-task-card__actions a { + min-height: 44px; + } +} + +@media (max-width: 359.98px) { + .mobile-task-card__header, + .mobile-task-card__dates, + .mobile-task-card__actions { + grid-template-columns: minmax(0, 1fr); + } + + .mobile-task-card__status { + max-width: none; + justify-content: flex-start; + text-align: left; + } +} diff --git a/src/app/projects/states/staff-notes/staff-notes.component.html b/src/app/projects/states/staff-notes/staff-notes.component.html index 13b472e7cb..81bd354b87 100644 --- a/src/app/projects/states/staff-notes/staff-notes.component.html +++ b/src/app/projects/states/staff-notes/staff-notes.component.html @@ -1,127 +1,202 @@ -
- @if (!loadingStaffNotes && project?.staffNoteCount === 0) { -
- No staff notes for {{ project.student.preferredName }} {{ project.student.lastName }} -
- } -
- @if (!loadingStaffNotes) { - @for (note of project?.staffNoteCache?.currentValues; track note) { - @if (note.replyToId) { -
-
- reply -
- @if (note.replyTo) { - - Replying to {{ note.replyTo.user.preferredName }} - {{ note.replyTo.user.lastName }} ({{ note.replyTo.user.nickname }}) - - {{ note.replyTo.note }} + +
+
+ @if (loadingStaffNotes) { +
+ +
+ } @else if (loadError) { +
+ + +
+ } @else if (notes.length === 0) { + + } @else { +
    + @for (note of notes; track note) { +
  1. +
    + +
    +
    +

    + + {{ note.user?.displayName }} + + + {{ note.createdAt | humanizedDate }} + +

    +
    + @if (note.authorIsMe) { + + } + + +
    +
    + + @if (note.replyToId) { + @if (note.replyTo) { + + } @else { +

    Replying to a deleted note

    + } + } + + @if (editingNote && editingNote.id === note.id) { +
    + + Edit note + + +
    + + +
    +
    } @else { - Replying to: Deleted note +
    }
    -
+ } - -
- @if (note.authorIsMe) { - edit - } - reply - delete -
-
- - - {{ note.user?.firstName }} {{ note.user?.lastName }} -
- {{ note.createdAt | humanizedDate }} -
-
- - - @if (editingNote && editingNote.id === note.id) { - - Update Note - -
- - -
- - } @else { -
- } - - - } - } @else { - + }
-
+
@if (replyingToNote) {
-
- reply -
- - Replying to {{ replyingToNote.user.firstName }} {{ replyingToNote.user.lastName }} ({{ - replyingToNote.user.nickname - }}) - - {{ replyingToNote.note }} -
+ +
+

+ Replying to {{ replyingToNote.user.displayName }} ({{ replyingToNote.user.nickname }}) +

+

{{ replyingToNote.note }}

- close +
} - - Staff Note + + Add a note -
- -
+ +
+ +
diff --git a/src/app/projects/states/staff-notes/staff-notes.component.scss b/src/app/projects/states/staff-notes/staff-notes.component.scss index a6d41ed2d2..6104c1173d 100644 --- a/src/app/projects/states/staff-notes/staff-notes.component.scss +++ b/src/app/projects/states/staff-notes/staff-notes.component.scss @@ -1,27 +1,52 @@ -.mat-icon { - color: #9696969d; - font-size: 20px; - width: 20px; - height: 20px; - cursor: pointer; - vertical-align: middle; - text-align: center; - margin-left: 0.3em; +// The card paints its surface here rather than with bg-ot-surface: Tailwind utilities are +// !important in this app, and an !important background would block the flash below. +.note-card { + background-color: var(--ot-color-surface); } -.mat-icon:hover { - color: black; +// Compact icon buttons for edit, reply and delete on each note card. +.note-action { + --mat-icon-button-state-layer-size: 32px; + --mat-icon-button-icon-size: 20px; + color: var(--ot-color-text-muted); } -@keyframes blueGlowFade { +.note-action:hover, +.note-action:focus-visible { + color: var(--ot-color-text); +} + +// The row used to be shown by a mouseover handler writing hoveredNoteId, which no keyboard +// user could ever set. The reveal is CSS now, so it answers to focus inside the note as +// well as to the pointer. It has to be opacity rather than display or visibility, both of +// which would drop the buttons out of the tab order and leave :focus-within with no way to +// ever become true. +.note-actions { + opacity: 0; +} + +.note-card:hover .note-actions, +.note-card:focus-within .note-actions { + opacity: 1; +} + +// A touch screen has no hover to reveal them with, so they stay in view there. +@media (hover: none) { + .note-actions { + opacity: 1; + } +} + +// Picking the note a reply points at scrolls to it and flashes it. +@keyframes note-flash { 0% { - background-color: rgba(66, 133, 244, 0.6); + background-color: var(--ot-color-selected); } 100% { - background-color: transparent; + background-color: var(--ot-color-surface); } } .flash-highlight { - animation: blueGlowFade 1s ease-out; + animation: note-flash 1s ease-out; } diff --git a/src/app/projects/states/staff-notes/staff-notes.component.ts b/src/app/projects/states/staff-notes/staff-notes.component.ts index 6ca071e75e..cf024f3228 100644 --- a/src/app/projects/states/staff-notes/staff-notes.component.ts +++ b/src/app/projects/states/staff-notes/staff-notes.component.ts @@ -3,9 +3,12 @@ import { Component, ElementRef, Input, + OnChanges, OnInit, + SimpleChanges, ViewChild, } from '@angular/core'; +import {Subscription} from 'rxjs'; import {Project, UserService} from 'src/app/api/models/doubtfire-model'; import {StaffNote} from 'src/app/api/models/staff-note'; import {StaffNoteService} from 'src/app/api/services/staff-note.service'; @@ -19,13 +22,15 @@ import {AlertService} from 'src/app/common/services/alert.service'; changeDetection: ChangeDetectionStrategy.Eager, standalone: false, }) -export class StaffNotesComponent implements OnInit { +export class StaffNotesComponent implements OnInit, OnChanges { @ViewChild('staffNotesContainer') staffNotesContainer!: ElementRef; @ViewChild('staffNoteEditor', {static: false}) staffNoteEditor!: ElementRef; @Input() project: Project; loadingStaffNotes: boolean = true; + /** The last load failed, so the list offers a retry instead of saying there are none. */ + loadError = false; noteText: string = ''; @@ -34,7 +39,7 @@ export class StaffNotesComponent implements OnInit { replyingToNote?: StaffNote; - hoveredNoteId: number | null = null; + private notesSub?: Subscription; constructor( private userService: UserService, @@ -43,11 +48,42 @@ export class StaffNotesComponent implements OnInit { private confirmationModalService: ConfirmationModalService, ) {} ngOnInit(): void { + this.loadNotes(); + } + + // The list reads the project's note cache, so a new project needs its notes loaded + // or it would claim there are none. A reply, an edit or a draft belongs to the old + // student, so none of them may carry over to the new one. + ngOnChanges(changes: SimpleChanges): void { + if (changes.project && !changes.project.firstChange && this.project) { + this.replyingToNote = null; + this.editingNote = null; + this.editingNoteText = ''; + this.noteText = ''; + this.loadNotes(); + } + } + + public get notes(): readonly StaffNote[] { + return this.project?.staffNoteCache?.currentValues ?? []; + } + + public loadNotes(): void { this.loadingStaffNotes = true; - this.staffNoteService.loadStaffNotes(this.project).subscribe((_notes) => { - this.loadingStaffNotes = false; - this.staffNoteService.updateStaffNoteReplies(this.project?.staffNoteCache.currentValues); - this.scrollDown(); + this.loadError = false; + // Drop a load still running for an earlier project, or it could land late and + // show that project's result over this one. + this.notesSub?.unsubscribe(); + this.notesSub = this.staffNoteService.loadStaffNotes(this.project).subscribe({ + next: (_notes) => { + this.loadingStaffNotes = false; + this.staffNoteService.updateStaffNoteReplies(this.project?.staffNoteCache.currentValues); + this.scrollDown(); + }, + error: () => { + this.loadingStaffNotes = false; + this.loadError = true; + }, }); } @@ -72,7 +108,7 @@ export class StaffNotesComponent implements OnInit { this.staffNoteService.addNote(this.project, noteText, this.replyingToNote).subscribe({ next: (_note) => { - this.alertService.success('Succesfully submitted note', 4000); + this.alertService.success('Successfully submitted note', 4000); this.scrollDown(); this.project.staffNoteCount++; this.replyingToNote = null; @@ -93,7 +129,7 @@ export class StaffNotesComponent implements OnInit { this.staffNoteService.updateNote(this.project, this.editingNote, noteText).subscribe({ next: (_note) => { - this.alertService.success('Succesfully updated note', 4000); + this.alertService.success('Successfully updated note', 4000); this.editingNote = null; this.editingNoteText = ''; }, diff --git a/src/app/projects/states/tutor-discussion/tutor-discussion.component.ts b/src/app/projects/states/tutor-discussion/tutor-discussion.component.ts index 8845bfecd1..5f771f9b62 100644 --- a/src/app/projects/states/tutor-discussion/tutor-discussion.component.ts +++ b/src/app/projects/states/tutor-discussion/tutor-discussion.component.ts @@ -1,19 +1,28 @@ -import {Html5QrcodeScanner, Html5QrcodeScannerState} from 'html5-qrcode'; +import { + Html5Qrcode, + Html5QrcodeCameraScanConfig, + Html5QrcodeScannerState, + Html5QrcodeSupportedFormats, +} from 'html5-qrcode'; import {DOCUMENT} from '@angular/common'; import { - AfterViewInit, ChangeDetectionStrategy, + ChangeDetectorRef, Component, + DestroyRef, Inject, Input, OnDestroy, + OnInit, ViewChild, - ViewEncapsulation, + inject, } from '@angular/core'; +import {takeUntilDestroyed} from '@angular/core/rxjs-interop'; import {MatDialog} from '@angular/material/dialog'; import {MatSelectionList} from '@angular/material/list'; import {MatTabChangeEvent} from '@angular/material/tabs'; -import {ActivatedRoute, Router} from '@angular/router'; +import {ActivatedRoute, ParamMap, Router, convertToParamMap} from '@angular/router'; +import {combineLatest, of} from 'rxjs'; import { AuthenticationService, Project, @@ -25,6 +34,7 @@ import { TaskStatusEnum, TutorialStream, Unit, + UnitRole, UnitService, UserService, } from 'src/app/api/models/doubtfire-model'; @@ -32,22 +42,101 @@ import {ConfirmationModalService} from 'src/app/common/modals/confirmation-modal import {DiscussedInClassReasonModalService} from 'src/app/common/modals/discussed-in-class-reason-modal/discussed-in-class-reason-modal.service'; import {AlertService} from 'src/app/common/services/alert.service'; import {GradeService} from 'src/app/common/services/grade.service'; +import {DoubtfireConstants} from 'src/app/config/constants/doubtfire-constants'; import {AddEngagementDialogComponent} from '../dashboard/directives/progress-dashboard/engagement-passport-card/add-engagement-dialog/add-engagement-dialog.component'; +import {GlobalStateService} from '../index/global-state.service'; enum TutorDiscussionTabView { SHOW_COMMENTS, SHOW_STAFF_NOTES, SHOW_DISCUSSION_PROMPTS, } + +/** Why the camera could not be used, so the page can say what to do about it. */ +export type CameraProblem = 'unsupported' | 'no-camera' | 'denied' | 'busy' | 'failed'; + +export const CAMERA_PROBLEMS: Record = { + 'unsupported': { + title: 'This browser cannot use a camera here', + detail: + 'Open this page in a recent version of Chrome, Edge, Firefox or Safari, over a secure connection.', + }, + 'no-camera': { + title: 'No camera found', + detail: 'Connect a camera, or open this page on a phone or tablet.', + }, + 'denied': { + title: 'Camera access is blocked', + detail: 'Allow the camera for this site in your browser settings, then try again.', + }, + 'busy': { + title: 'The camera is in use', + detail: 'Another app or tab is using it. Close that, then try again.', + }, + 'failed': { + title: 'The camera did not start', + detail: 'Try again. If it keeps failing, reload the page.', + }, +}; + +/** + * Sort a camera error into something the tutor can act on. The scanner hands back the + * browser's own error, or a string with the error's name inside it. + */ +export function cameraProblemFrom(error: unknown): CameraProblem { + const details = error as {name?: string; message?: string} | null; + const text = `${details?.name ?? ''} ${details?.message ?? error}`; + if (/NotAllowed|Permission|SecurityError/i.test(text)) { + return 'denied'; + } + if (/NotFound|DevicesNotFound|Overconstrained|not found/i.test(text)) { + return 'no-camera'; + } + if (/NotReadable|TrackStart|Could not start|in use/i.test(text)) { + return 'busy'; + } + if (/not supported/i.test(text)) { + return 'unsupported'; + } + return 'failed'; +} + +/** The part of the camera scanner this page uses, so a spec can hand it a stand-in. */ +export interface QrScanner { + start( + camera: string | MediaTrackConstraints, + config: Html5QrcodeCameraScanConfig, + onScan: (decodedText: string) => void, + onScanFailure: () => void, + ): Promise; + stop(): Promise; + pause(shouldPauseVideo?: boolean): void; + resume(): void; + clear(): void; + getState(): Html5QrcodeScannerState; + getRunningTrackSettings(): MediaTrackSettings; +} + +export interface CameraOption { + id: string; + label: string; +} + +const QR_READER_ID = 'qr-reader'; +// The scanner keeps its own preference under this key. Reuse it, so the camera a tutor +// picked before this page changed is still the one it opens with. +const CAMERA_STORAGE_KEY = 'HTML5_QRCODE_DATA'; +const NOT_IN_UNIT = 'That student is not enrolled in this unit.'; +const NOT_A_STUDENT_CODE = "That QR code is not a student's code."; + @Component({ selector: 'f-tutor-discussion', templateUrl: './tutor-discussion.component.html', styleUrl: './tutor-discussion.component.scss', - encapsulation: ViewEncapsulation.None, changeDetection: ChangeDetectionStrategy.Eager, standalone: false, }) -export class TutorDiscussionComponent implements AfterViewInit, OnDestroy { +export class TutorDiscussionComponent implements OnInit, OnDestroy { private readonly discussedInClassNotePrefix = `I'm manually marking this discussed in class because...`; private readonly mobileDiscussionViewportContent = 'width=device-width, initial-scale=0.8, maximum-scale=5'; @@ -61,23 +150,50 @@ export class TutorDiscussionComponent implements AfterViewInit, OnDestroy { public filteredTasks: Task[] = []; public allTasks: Task[] = []; + public showingAllSubmitted = false; public unit: Unit | null; public project: Project | null; public selectedTask: Task | null; - public allowHover = true; - public isNarrow = false; + /** The camera view is on screen. */ public scanningQr: boolean = false; + /** Waiting for the camera to start, which includes the browser's permission prompt. */ + public cameraStarting = false; public loadingStudentData: boolean = false; - - private html5QrcodeScanner?: Html5QrcodeScanner; + public loadingUnit = false; + + public cameraProblem: CameraProblem | null = null; + public readonly cameraProblems = CAMERA_PROBLEMS; + /** A note under the camera view, such as a code that is not a student's. */ + public scanHint: string | null = null; + /** Why the page could not open the unit or the student it was asked for. */ + public loadError: string | null = null; + + public cameras: CameraOption[] = []; + public selectedCameraId: string | null = null; + public studentLookup = ''; + + public readonly externalName = inject(DoubtfireConstants).ExternalName; + + private qrScanner?: QrScanner; + // The scanner whose start() has not settled yet. Its own start flow releases it, so + // nothing else may stop or clear it while the camera is coming up. + private startingScanner?: QrScanner; + // Bumped whenever the page moves on, so a late answer to an older request is dropped + // instead of replacing what the tutor has opened since. + private loadGeneration = 0; + private unitLoadGeneration = 0; private originalViewportContent: string | null = null; private mobileDiscussionZoomApplied = false; + private readonly destroyRef = inject(DestroyRef); + private destroyed = false; private _unitId: number; - private _username: string; + private _username: string | null; + private _projectId: number | null = null; + private pageKey: string | null = null; public TutorDiscussionTabView = TutorDiscussionTabView; public footerTabView: TutorDiscussionTabView = TutorDiscussionTabView.SHOW_COMMENTS; @@ -97,24 +213,98 @@ export class TutorDiscussionComponent implements AfterViewInit, OnDestroy { private taskCommentService: TaskCommentService, private taskService: TaskService, private dialog: MatDialog, + private globalState: GlobalStateService, + private changeDetector: ChangeDetectorRef, ) {} + public ngOnInit(): void { + this.attendance = + this.attendance ?? + this.activatedRoute.snapshot.data.attendance ?? + this.activatedRoute.snapshot.queryParamMap.get('attendance') === 'true'; + + // The router keeps this page when only the unit in the url or the query changes, for + // example when a tutor moves to another unit's Discussion from the menu, or opens a + // second student's code link. Follow both, so the page never shows the last one. + const parentParams = this.activatedRoute.parent?.paramMap ?? of(convertToParamMap({})); + combineLatest([parentParams, this.activatedRoute.queryParamMap]) + .pipe(takeUntilDestroyed(this.destroyRef)) + .subscribe(([params, query]) => this.openFromRoute(params, query)); + } + public ngOnDestroy(): void { + this.destroyed = true; this.stopQrScanner(); this.restoreViewportZoom(); } + /** The units this person teaches now, offered when the page has no unit to work in. */ + public get teachingUnits(): UnitRole[] { + return (this.globalState.loadedUnitRoles?.currentValues ?? []).filter( + (unitRole) => unitRole?.unit?.isActive, + ); + } + + public get hasUnit(): boolean { + return !!this._unitId; + } + + public get cameraSupported(): boolean { + return ( + typeof navigator !== 'undefined' && + !!navigator.mediaDevices?.getUserMedia && + window.isSecureContext !== false + ); + } + + public get canStartScanning(): boolean { + return ( + !this.cameraStarting && + !this.loadingStudentData && + this.cameraProblem !== 'unsupported' && + (!this.attendance || !!this.selectedTaskDefinition) + ); + } + + public get canFindStudent(): boolean { + return ( + this.hasUnit && + this.studentLookup.trim().length > 0 && + !this.loadingStudentData && + (!this.attendance || !!this.selectedTaskDefinition) + ); + } + + public get selectedTaskCount(): number { + return this.selectedCount(this.tasksList); + } + + public selectedCount(list?: MatSelectionList): number { + return list?.selectedOptions?.selected.length ?? 0; + } + public currentUserTutorsInStream(tutorialStream: TutorialStream): boolean { const user = this.userService.currentUser; - const tutorials = this.unit.tutorials.filter( + if (!tutorialStream || !user) { + return false; + } + // A tutorial can have no stream or no tutor, so guard both rather than throw while + // the task list renders. + return (this.unit?.tutorials ?? []).some( (t) => - t.tutorialStream.abbreviation === tutorialStream.abbreviation && - t.tutorialStream.name === tutorialStream.name, + t.tutorialStream?.abbreviation === tutorialStream.abbreviation && + t.tutorialStream?.name === tutorialStream.name && + t.tutor?.id === user.id, + ); + } + + /** Tasks the tutor most likely wants to act on start out ticked. */ + public isPreselected(task: Task): boolean { + return ( + (['discuss', 'rediscuss'].includes(task.status) || !!this.attendance) && + (!task.definition?.lockAssessmentsToTutorialStream || + this.currentUserTutorsInStream(task.definition.tutorialStream)) ); - if (tutorials.some((t) => t.tutor.id === user.id)) { - return true; - } - return false; } onTabChange(event: MatTabChangeEvent): void { @@ -139,108 +329,156 @@ export class TutorDiscussionComponent implements AfterViewInit, OnDestroy { this.footerTabView = TutorDiscussionTabView.SHOW_DISCUSSION_PROMPTS; } - public ngAfterViewInit(): void { - this.unitId = - this.unitId ?? - Number( - this.activatedRoute.parent?.snapshot.paramMap.get('unitId') ?? - this.activatedRoute.snapshot.queryParamMap.get('unitId'), - ); - this.username = this.username ?? this.activatedRoute.snapshot.queryParamMap.get('username'); - this.attendance = - this.attendance ?? - this.activatedRoute.snapshot.data.attendance ?? - this.activatedRoute.snapshot.queryParamMap.get('attendance') === 'true'; + private openFromRoute(params: ParamMap, query: ParamMap): void { + const unitId = Number(this.unitId ?? params.get('unitId') ?? query.get('unitId')) || null; + const username = this.username ?? query.get('username'); + const key = `${unitId}|${username ?? ''}`; + if (key === this.pageKey) { + return; + } + const isFirstOpen = this.pageKey === null; + this.pageKey = key; + if (!isFirstOpen) { + this.resetPage(); + } this.authService.afterAuthCall((result) => { if (!result) { return this.router.navigateByUrl('/sign_in'); + } + if (this.userService.currentUser.systemRole === 'Student') { + // Nothing here is for a student, and the guard is about to send them away. + return; + } + if (!this.cameraSupported) { + this.cameraProblem = 'unsupported'; + } else if (isFirstOpen) { + this.watchCameraPermission(); + } + if (!unitId) { + return; + } + this._unitId = unitId; + if (username && !this.attendance) { + this._username = username; + this._projectId = null; + this.getStudentTasks(); } else { - if (this.userService.currentUser.systemRole === 'Student') { - // Avoid prompting students for camera permissions before redirecting to unauthorised state - return; - } - if (this.unitId) { - this._unitId = Number(this.unitId); - if (!this.attendance) { - // Tutor discussion view - if (this.username) { - this._username = this.username; - this.getStudentTasks(); - } else { - setTimeout(() => this.scanQrCode()); - } - } else { - this.getUnit().then((u) => { - this.unit = u; - }); - } - } + this.loadUnit(); } }); } + private resetPage(): void { + this.loadGeneration++; + this.unitLoadGeneration++; + this.loadingUnit = false; + this.stopQrScanner(); + this.restoreViewportZoom(); + this.scanningQr = false; + this.loadingStudentData = false; + this.unit = null; + this.project = null; + this.selectedTask = null; + this.selectedTaskDefinition = null; + this.filteredTasks = []; + this.allTasks = []; + this.showingAllSubmitted = false; + this.loadError = null; + this.scanHint = null; + this._unitId = undefined; + this._username = null; + this._projectId = null; + } + + private loadUnit(): void { + const generation = ++this.unitLoadGeneration; + const isCurrent = () => generation === this.unitLoadGeneration && !this.destroyed; + this.loadingUnit = true; + this.loadError = null; + this.getUnit() + .then((unit) => { + // A student opened in the meantime brings their own copy of the unit. + if (isCurrent() && !this.project) { + this.unit = unit; + } + }) + .catch(() => { + if (isCurrent()) { + this.loadError = 'This unit could not be loaded. Reload the page to try again.'; + } + }) + .finally(() => { + if (isCurrent()) { + this.loadingUnit = false; + } + }); + } + private decodeQrCode(data: string) { if (!this.scanningQr || this.loadingStudentData) { return; } + let params: URLSearchParams; try { - const params = new URL(data).searchParams; - const unitId = parseInt(params.get('unitId')); - const projectId = parseInt(params.get('projectId')); - const username = params.get('username'); - - if ((!isNaN(unitId) && !isNaN(projectId)) || username) { - if (unitId) { - this._unitId = unitId; - } - if (username) { - this._username = username; - } - - this.changeProject(); - } + params = new URL(data).searchParams; } catch { - // QR code data is invalid + this.scanHint = NOT_A_STUDENT_CODE; + return; + } + + const unitId = parseInt(params.get('unitId')); + const projectId = parseInt(params.get('projectId')); + const username = params.get('username'); + + if ((isNaN(unitId) || isNaN(projectId)) && !username) { + this.scanHint = NOT_A_STUDENT_CODE; + return; + } + + // Check-in records a task from this unit, so a code from another unit cannot count. + if (this.attendance && this.unit && !isNaN(unitId) && unitId !== this.unit.id) { + this.scanHint = "That student's code is for a different unit."; + return; + } + + this.scanHint = null; + if (unitId) { + this._unitId = unitId; } + // A code with only a project id used to fall back on whichever student was scanned + // last. Look it up by the project instead. + this._username = username || null; + this._projectId = username || isNaN(projectId) ? null : projectId; + + this.changeProject(); } + /** Close the camera and go back to the page, keeping any student already open. */ public closeQrReader(): void { - if (!this.project) { - // Exiting the route entirely - this.stopQrScanner(); - if (this.unitId) { - this.router.navigate(['/units', this.unitId, 'tasks', 'inbox']); - } else { - this.router.navigateByUrl('/home'); - } - } else { - // Close the camera view - this.scanningQr = false; - this.stopQrScanner(); - } + this.scanningQr = false; + this.scanHint = null; + this.stopQrScanner(); } private changeProject() { - this.html5QrcodeScanner?.pause(true); - this.loadingStudentData = true; - setTimeout(() => { - try { - this.getStudentTasks(); - } catch (_e) { - this.alertService.error(`Invalid QR code`, 2000); - this.loadingStudentData = false; + this.pauseScanner(); + this.getStudentTasks(); + } - setTimeout(() => { - this.html5QrcodeScanner?.resume(); - }, 2000); - } - }); + /** Look a student up by username or student id, for when there is no camera. */ + public findStudent(): void { + if (!this.canFindStudent) { + return; + } + this._username = this.studentLookup.trim(); + this._projectId = null; + this.getStudentTasks(); } private applyMobileDiscussionZoom(): void { - if (!window.matchMedia('(max-width: 768px)').matches) { + if (!window.matchMedia?.('(max-width: 768px)').matches) { return; } @@ -267,114 +505,283 @@ export class TutorDiscussionComponent implements AfterViewInit, OnDestroy { this.mobileDiscussionZoomApplied = false; } - hideQrScannerBloat: boolean = true; + /** Builds the camera scanner. A spec replaces this with a stand-in. */ + protected createQrScanner(elementId: string): QrScanner { + return new Html5Qrcode(elementId, { + verbose: false, + formatsToSupport: [Html5QrcodeSupportedFormats.QR_CODE], + }); + } private async stopQrScanner(): Promise { - if (!this.html5QrcodeScanner) { + const scanner = this.qrScanner; + this.qrScanner = undefined; + if (!scanner || scanner === this.startingScanner) { + // A camera that is still starting is let go by its own start flow once it settles. return; } + await this.releaseScanner(scanner); + } + private async releaseScanner(scanner: QrScanner): Promise { try { - await this.html5QrcodeScanner.clear(); + const state = scanner.getState(); + if (state === Html5QrcodeScannerState.SCANNING || state === Html5QrcodeScannerState.PAUSED) { + await scanner.stop(); + } + scanner.clear(); } catch (_e) { - // The scanner may already be stopped by its own controls. - } finally { - this.html5QrcodeScanner = undefined; + // The camera may already be closed. Either way it is let go. } } - private async getCameraPermissionState(): Promise { - if (!navigator.permissions?.query) { - return null; + private pauseScanner(): void { + try { + if (this.qrScanner?.getState() === Html5QrcodeScannerState.SCANNING) { + this.qrScanner.pause(true); + } + } catch (_e) { + // Not scanning, so there is nothing to pause. } + } + private resumeScanner(): void { try { - const permissionStatus = await navigator.permissions.query({ - name: 'camera' as PermissionName, - }); - return permissionStatus.state; + if (this.qrScanner?.getState() === Html5QrcodeScannerState.PAUSED) { + this.qrScanner.resume(); + } + } catch (_e) { + // Resuming failed, so start the camera over. + this.scanQrCode(); + } + } + + private rememberedCameraId(): string | null { + try { + return ( + JSON.parse(localStorage.getItem(CAMERA_STORAGE_KEY) ?? 'null')?.lastUsedCameraId ?? null + ); } catch (_e) { return null; } } - private async prepareQrScannerCamera(): Promise { - const cachedScannerData = localStorage.getItem('HTML5_QRCODE_DATA'); - const cameraPermissionState = await this.getCameraPermissionState(); - if (cachedScannerData) { - try { - const html5QrcodeData = JSON.parse(cachedScannerData); - if (html5QrcodeData?.hasPermission && cameraPermissionState === 'granted') { - this.hideQrScannerBloat = html5QrcodeData.lastUsedCameraId ? true : false; + private rememberCamera(cameraId: string | null): void { + try { + if (cameraId) { + localStorage.setItem( + CAMERA_STORAGE_KEY, + JSON.stringify({hasPermission: true, lastUsedCameraId: cameraId}), + ); + } else { + localStorage.removeItem(CAMERA_STORAGE_KEY); + } + } catch (_e) { + // Storage can be unavailable, for example in a private window. Nothing to keep. + } + } + + /** + * Start the camera and scan for a student's code. The browser only asks for the camera + * here, when the tutor has chosen to scan, never when the page opens. + */ + public async scanQrCode(): Promise { + if (this.attendance && !this.selectedTaskDefinition) { + this.alertService.error('Choose a task to check in first', 3000); + return; + } + if (!this.cameraSupported) { + this.cameraProblem = 'unsupported'; + return; + } + if (this.cameraStarting) { + return; + } + + this.cameraProblem = null; + this.scanHint = null; + this.loadError = null; + this.loadingStudentData = false; + this.scanningQr = true; + + if (this.qrScanner?.getState() === Html5QrcodeScannerState.PAUSED) { + this.resumeScanner(); + return; + } + + // Claim the camera before the first wait, so nothing else can start one while the + // old scanner is still being stopped. + this.cameraStarting = true; + try { + await this.stopQrScanner(); + if (this.destroyed || !this.scanningQr) { + return; + } + // Draw the camera view first: the scanner sizes the video to the element it is given. + this.changeDetector.detectChanges(); + await this.startCamera(this.selectedCameraId ?? this.rememberedCameraId()); + } finally { + this.cameraStarting = false; + } + } + + /** + * Start the camera. Only one start runs at a time: cameraStarting stays true until this + * one has settled and cleaned up after itself, and the page offers no way to start or + * switch cameras until then. Two starts at once would both draw into #qr-reader, and + * the older one's clean up would wipe the newer one's video. + */ + private async startCamera(cameraId: string | null): Promise { + this.cameraStarting = true; + try { + // Try the camera used last time first. It may be gone, so fall back on letting the + // browser choose, preferring a camera that faces away from the tutor. + const attempts = cameraId ? [cameraId, null] : [null]; + for (const [index, attempt] of attempts.entries()) { + const outcome = await this.startCameraWith(attempt); + if (outcome === 'started' || outcome === 'cancelled') { return; } - } catch (_e) { - localStorage.removeItem('HTML5_QRCODE_DATA'); + const isLastAttempt = index === attempts.length - 1; + if (outcome === 'denied' || isLastAttempt) { + this.scanningQr = false; + this.cameraProblem = outcome; + return; + } + this.rememberCamera(null); + this.selectedCameraId = null; } + } finally { + this.cameraStarting = false; } + } - // Trigger video permissions once so device labels are available for back camera selection. - // Stopping these tracks releases the camera; the browser keeps the permission grant. - const stream = await navigator.mediaDevices.getUserMedia({video: true}); + private async startCameraWith( + cameraId: string | null, + ): Promise<'started' | 'cancelled' | CameraProblem> { + if (this.destroyed || !this.scanningQr) { + return 'cancelled'; + } + let scanner: QrScanner; try { - const devices = await navigator.mediaDevices.enumerateDevices(); + scanner = this.createQrScanner(QR_READER_ID); + } catch (_e) { + // The camera view is not on the page, so there is nothing to draw into. + return this.destroyed ? 'cancelled' : 'failed'; + } - // Find the deviceId of the back camera - const backCameras = devices.filter( - (d) => d.kind === 'videoinput' && d.label.toLowerCase().includes('back camera'), + this.qrScanner = scanner; + this.startingScanner = scanner; + let problem: CameraProblem | null = null; + try { + await scanner.start( + cameraId ?? {facingMode: 'environment'}, + {fps: 10, qrbox: (width, height) => this.viewfinderBox(width, height)}, + (decodedText) => this.decodeQrCode(decodedText), + () => { + // Most frames hold no code at all. That is not an error worth showing. + }, ); + } catch (error) { + problem = cameraProblemFrom(error); + } + this.startingScanner = undefined; - const html5QrcodeData = { - hasPermission: true, - lastUsedCameraId: backCameras[0]?.deviceId ?? null, - }; - localStorage.setItem('HTML5_QRCODE_DATA', JSON.stringify(html5QrcodeData)); + const stillWanted = !this.destroyed && this.scanningQr && this.qrScanner === scanner; + if (problem || !stillWanted) { + if (this.qrScanner === scanner) { + this.qrScanner = undefined; + } + // The tutor stopped, or left, while the camera was starting, or it failed. + await this.releaseScanner(scanner); + return stillWanted ? problem : 'cancelled'; + } - // Hide most of the UI if we found and set the back camera - // Otherwise, we need to reveal the UI so that the user can select which camera to use - this.hideQrScannerBloat = html5QrcodeData.lastUsedCameraId ? true : false; - } finally { - stream.getTracks().forEach((track) => track.stop()); + await this.listCameras(scanner); + return 'started'; + } + + private viewfinderBox(width: number, height: number): {width: number; height: number} { + const size = Math.max(50, Math.floor(Math.min(width, height) * 0.7)); + return {width: size, height: size}; + } + + /** Once the camera runs its devices have names, so offer a choice if there is one. */ + private async listCameras(scanner: QrScanner): Promise { + try { + const devices = await navigator.mediaDevices.enumerateDevices(); + this.cameras = devices + .filter((device) => device.kind === 'videoinput' && device.deviceId) + .map((device, index) => ({ + id: device.deviceId, + label: device.label || `Camera ${index + 1}`, + })); + } catch (_e) { + this.cameras = []; + } + + try { + this.selectedCameraId = scanner.getRunningTrackSettings()?.deviceId ?? this.selectedCameraId; + } catch (_e) { + // The camera closed again before its settings could be read. } } - public scanQrCode() { - if (this.attendance && !this.selectedTaskDefinition) { - this.alertService.error('You must select a task first', 3000); + /** + * Say up front when the browser has already been told to block the camera. This only + * reads the permission, it never asks for it, and it clears once the tutor allows it. + */ + private async watchCameraPermission(): Promise { + if (!navigator.permissions?.query) { return; } - this.scanningQr = true; - this.loadingStudentData = false; + try { + const status = await navigator.permissions.query({name: 'camera' as PermissionName}); + if (this.destroyed) { + return; + } + const update = () => { + if (status.state === 'denied' && !this.scanningQr) { + this.cameraProblem = 'denied'; + } else if (status.state !== 'denied' && this.cameraProblem === 'denied') { + this.cameraProblem = null; + } + }; + update(); + // A listener rather than onchange, so the change runs inside the zone and the + // page updates when the tutor flips the setting. + status.addEventListener('change', update); + this.destroyRef.onDestroy(() => status.removeEventListener('change', update)); + } catch (_e) { + // Some browsers cannot report the camera permission. The camera will tell us instead. + } + } - if (this.html5QrcodeScanner?.getState() === Html5QrcodeScannerState.PAUSED) { - this.html5QrcodeScanner.resume(); - } else { - this.stopQrScanner() - .then(() => this.prepareQrScannerCamera()) - .then(() => { - setTimeout(() => { - this.html5QrcodeScanner = new Html5QrcodeScanner( - 'qr-reader', // id of the div in the html - {fps: 10, qrbox: 250}, - false, - ); - - this.html5QrcodeScanner.render( - (data) => { - this.decodeQrCode(data); - }, - (_error) => { - // console.error(_error); - }, - ); - }); - }) - .catch((_e) => { - this.scanningQr = false; - this.alertService.error('Camera permission is required to scan QR codes', 3000); - }); + public async switchCamera(cameraId: string): Promise { + if (!cameraId || cameraId === this.selectedCameraId || this.cameraStarting) { + return; + } + this.selectedCameraId = cameraId; + this.rememberCamera(cameraId); + // Claim the camera before the first wait, so a second switch cannot slip in while + // this one is still stopping the old camera. + this.cameraStarting = true; + try { + await this.stopQrScanner(); + if (this.scanningQr && !this.destroyed) { + await this.startCamera(cameraId); + } + } finally { + this.cameraStarting = false; + } + } + + public clearCheckInTask(): void { + this.selectedTaskDefinition = null; + if (this.scanningQr) { + this.closeQrReader(); } } @@ -394,6 +801,7 @@ export class TutorDiscussionComponent implements AfterViewInit, OnDestroy { public loadTaskComments(event: MouseEvent, task: Task) { event.stopPropagation(); this.selectedTask = task; + this.showComments(); } public async setSelectedTasksStatus(status: TaskStatusEnum) { @@ -471,7 +879,11 @@ export class TutorDiscussionComponent implements AfterViewInit, OnDestroy { } public get canMarkSelectedTasksComplete(): boolean { - const selectedTasks = this.tasksList?.selectedOptions?.selected ?? []; + return this.canCompleteSelection(this.tasksList); + } + + public canCompleteSelection(list?: MatSelectionList): boolean { + const selectedTasks = list?.selectedOptions?.selected ?? []; if (!selectedTasks.length) { return false; } @@ -483,7 +895,11 @@ export class TutorDiscussionComponent implements AfterViewInit, OnDestroy { } public get selectedTasksIncludeDiscuss(): boolean { - const selectedTasks = this.tasksList?.selectedOptions?.selected ?? []; + return this.selectionIncludesDiscuss(this.tasksList); + } + + public selectionIncludesDiscuss(list?: MatSelectionList): boolean { + const selectedTasks = list?.selectedOptions?.selected ?? []; return selectedTasks.some((taskOption) => { const task = taskOption.value as Task; return task.status === 'discuss'; @@ -556,25 +972,47 @@ export class TutorDiscussionComponent implements AfterViewInit, OnDestroy { }); } + private isRequestedStudent(project: Project): boolean { + if (this._username) { + const wanted = this._username.trim().toLowerCase(); + const student = project?.student; + return ( + student?.username?.toLowerCase() === wanted || + `${student?.studentId ?? ''}`.toLowerCase() === wanted + ); + } + return this._projectId != null && project?.id === this._projectId; + } + private loadStudents(unit: Unit): Promise { return new Promise((resolve, reject) => { - this.projectService.loadStudents(unit, false, false).subscribe((projects) => { - const project = projects.find((p) => p.student.username === this._username); - if (!project) { - reject('Student is not a part of this unit'); - } - resolve(project); + this.projectService.loadStudents(unit, false, false).subscribe({ + next: (projects) => { + const project = projects.find((p) => this.isRequestedStudent(p)); + if (project) { + resolve(project); + } else { + reject(NOT_IN_UNIT); + } + }, + // Without this the promise never settled on a failed request, and the page sat + // on its loading state for good. + error: (error) => reject(error), }); }); } private getProject(unit: Unit, projectId: number): Promise { return new Promise((resolve, reject) => { - this.projectService.loadProject(projectId, unit, true).subscribe((project) => { - if (!project) { - reject('No project found'); - } - resolve(project); + this.projectService.loadProject(projectId, unit, true).subscribe({ + next: (project) => { + if (project) { + resolve(project); + } else { + reject('That student could not be loaded.'); + } + }, + error: (error) => reject(error), }); }); } @@ -583,10 +1021,6 @@ export class TutorDiscussionComponent implements AfterViewInit, OnDestroy { return this.gradeService.gradeLabel(grade, this.project?.unit); } - public refresh() { - this.decodeQrCode('{"unitId":2,"projectId":20}'); - } - statusesToInclude: TaskStatusEnum[] = [ 'demonstrate', 'ready_for_feedback', @@ -601,6 +1035,7 @@ export class TutorDiscussionComponent implements AfterViewInit, OnDestroy { public viewAllSubmittedTasks() { this.filteredTasks = [...this.allTasks]; + this.showingAllSubmitted = true; } private filteredDiscussionTasks(tasks: readonly Task[]): Task[] { @@ -623,23 +1058,36 @@ export class TutorDiscussionComponent implements AfterViewInit, OnDestroy { public viewAllFilteredTasks() { const discussionTasks = this.filteredDiscussionTasks(this.project?.tasks ?? []); this.filteredTasks = [...discussionTasks]; + this.showingAllSubmitted = false; + } + + /** There are submitted tasks beyond the ones waiting to be discussed. */ + public get hasMoreSubmittedTasks(): boolean { + return this.allTasks.length > this.filteredDiscussionTasks(this.allTasks).length; } public getStudentTasks(): void { - console.time('getStudentTasks()'); - // this.project = null; - // this.filteredTasks = []; - // this.selectedTask = null; + const generation = ++this.loadGeneration; + const isCurrent = () => generation === this.loadGeneration && !this.destroyed; + this.loadError = null; + this.loadingStudentData = true; + const wasScanning = this.scanningQr; this.getUnit() .then((_unit) => { + if (!isCurrent()) { + return null; + } this.unit = _unit; return this.loadStudents(this.unit); }) .then((student) => { - return this.getProject(this.unit, student.id); + return student && isCurrent() ? this.getProject(this.unit, student.id) : null; }) .then((project) => { + if (!project || !isCurrent()) { + return; + } const discussionTasks = this.filteredDiscussionTasks(project.tasks); if (!this.attendance) { this.filteredTasks = [...discussionTasks]; @@ -647,29 +1095,41 @@ export class TutorDiscussionComponent implements AfterViewInit, OnDestroy { ...project.tasks.filter( (task) => task.status !== 'not_started' && // Filter out tasks with no submissions yet - task.definition.targetGrade <= project.targetGrade, // Filter out tasks that are higher than student's target grade + task.definition?.targetGrade <= project.targetGrade, // Filter out tasks that are higher than student's target grade ), ]; } else { - this.filteredTasks = [ - project.tasks.find((t) => t.definition.id === this.selectedTaskDefinition.id), - ]; + const task = project.tasks.find( + (t) => t.definition?.id === this.selectedTaskDefinition?.id, + ); + this.filteredTasks = task ? [task] : []; + this.allTasks = []; } + this.showingAllSubmitted = false; this.selectedTask = this.filteredTasks[0] ?? null; this.project = project; + this.studentLookup = ''; this.scanningQr = false; + this.scanHint = null; this.loadingStudentData = false; this.stopQrScanner(); this.applyMobileDiscussionZoom(); }) .catch((e) => { - console.error(e); - this.alertService.error(e, 5000); - this.scanQrCode(); - }) - .finally(() => { - console.timeEnd('getStudentTasks()'); + if (!isCurrent()) { + return; + } + this.loadingStudentData = false; + const message = + typeof e === 'string' && e.length > 0 ? e : 'That student could not be loaded.'; + if (wasScanning && this.scanningQr) { + // Keep scanning, and say why this code did not open a student. + this.scanHint = message; + this.resumeScanner(); + } else { + this.loadError = message; + } }); } } diff --git a/src/app/projects/states/tutor-notes/tutor-notes.component.html b/src/app/projects/states/tutor-notes/tutor-notes.component.html index ec2c4581d4..a30ce2f6da 100644 --- a/src/app/projects/states/tutor-notes/tutor-notes.component.html +++ b/src/app/projects/states/tutor-notes/tutor-notes.component.html @@ -1,175 +1,271 @@ -
- -
- @if (!loadingTutorNotes) { - @for (note of filteredNotes; track note) { - @if (note.replyToId) { -
-
- reply -
- @if (note.replyTo) { - - Replying to {{ note.replyTo.user.preferredName }} - {{ note.replyTo.user.lastName }} ({{ note.replyTo.user.nickname }}) - - {{ note.replyTo.note }} + +
+
+ @if (loadingTutorNotes) { +
+ +
+ } @else if (loadError) { +
+ + +
+ } @else if (allNotes.length === 0) { + + } @else if (filteredNotes.length === 0) { + + } @else { +
    + @for (note of filteredNotes; track note) { +
  1. +
    + +
    +
    +

    + + {{ note.user?.displayName }} + + + {{ note.createdAt | humanizedDate }} + +

    +
    + @if (note.authorIsMe) { + + } + + +
    +
    + + @if (note.replyToId) { + @if (note.replyTo) { + + } @else { +

    Replying to a deleted note

    + } + } + + @if (editingNote && editingNote.id === note.id) { +
    + + Edit note + + +
    + + +
    +
    } @else { - Replying to: Deleted note +
    + } + + @if ( + note.taskDefinition || note.project || note.readByUnitRole || note.noteIsForMe + ) { +
    +
    + @if (note.taskDefinition) { + + {{ note.taskDefinition?.abbreviation }} {{ note.taskDefinition.name }} + + } + @if (note.project) { + + {{ note.project?.student.displayName }} + + } +
    + @if (!note.readByUnitRole) { + @if (note.noteIsForMe) { + + } + } @else { + Read by tutor + } +
    }
    -
+ } - -
-
- @if (note.authorIsMe) { - edit - } - reply - delete -
- @if (!note.readByUnitRole) { - @if (note.noteIsForMe) { - - } - } @else { -
Read by tutor
- } -
- -
- - - {{ note.user?.firstName }} {{ note.user?.lastName }} -
- {{ note.createdAt | humanizedDate }} -
-
- -
- @if (note.taskDefinition) { - {{ note.taskDefinition?.abbreviation }} {{ note.taskDefinition.name }} - } - @if (note.project) { - {{ note.project?.student.name }} - } -
- - - @if (editingNote && editingNote.id === note.id) { - - Update Note - -
- - -
- - } @else { -
- } - - - } - } @else { - + }
-
- - All Tasks - @for (option of taskDefinitionFilters; track option) { +
+ + @if (allNotes.length > 0) { + All tasks - {{ option }} - - } - + @for (option of taskDefinitionFilters; track option) { + + {{ option }} + + } + + } @if (replyingToNote) {
-
- reply -
- - Replying to {{ replyingToNote.user.firstName }} {{ replyingToNote.user.lastName }} ({{ - replyingToNote.user.nickname - }}) - - {{ replyingToNote.note }} -
+ +
+

+ Replying to {{ replyingToNote.user.displayName }} ({{ replyingToNote.user.nickname }}) +

+

{{ replyingToNote.note }}

- close +
} - - Tutor note + + Add a note -
- -
+ +
+ +
diff --git a/src/app/projects/states/tutor-notes/tutor-notes.component.scss b/src/app/projects/states/tutor-notes/tutor-notes.component.scss index a6d41ed2d2..6104c1173d 100644 --- a/src/app/projects/states/tutor-notes/tutor-notes.component.scss +++ b/src/app/projects/states/tutor-notes/tutor-notes.component.scss @@ -1,27 +1,52 @@ -.mat-icon { - color: #9696969d; - font-size: 20px; - width: 20px; - height: 20px; - cursor: pointer; - vertical-align: middle; - text-align: center; - margin-left: 0.3em; +// The card paints its surface here rather than with bg-ot-surface: Tailwind utilities are +// !important in this app, and an !important background would block the flash below. +.note-card { + background-color: var(--ot-color-surface); } -.mat-icon:hover { - color: black; +// Compact icon buttons for edit, reply and delete on each note card. +.note-action { + --mat-icon-button-state-layer-size: 32px; + --mat-icon-button-icon-size: 20px; + color: var(--ot-color-text-muted); } -@keyframes blueGlowFade { +.note-action:hover, +.note-action:focus-visible { + color: var(--ot-color-text); +} + +// The row used to be shown by a mouseover handler writing hoveredNoteId, which no keyboard +// user could ever set. The reveal is CSS now, so it answers to focus inside the note as +// well as to the pointer. It has to be opacity rather than display or visibility, both of +// which would drop the buttons out of the tab order and leave :focus-within with no way to +// ever become true. +.note-actions { + opacity: 0; +} + +.note-card:hover .note-actions, +.note-card:focus-within .note-actions { + opacity: 1; +} + +// A touch screen has no hover to reveal them with, so they stay in view there. +@media (hover: none) { + .note-actions { + opacity: 1; + } +} + +// Picking the note a reply points at scrolls to it and flashes it. +@keyframes note-flash { 0% { - background-color: rgba(66, 133, 244, 0.6); + background-color: var(--ot-color-selected); } 100% { - background-color: transparent; + background-color: var(--ot-color-surface); } } .flash-highlight { - animation: blueGlowFade 1s ease-out; + animation: note-flash 1s ease-out; } diff --git a/src/app/projects/states/tutor-notes/tutor-notes.component.spec.ts b/src/app/projects/states/tutor-notes/tutor-notes.component.spec.ts new file mode 100644 index 0000000000..071f9e4e11 --- /dev/null +++ b/src/app/projects/states/tutor-notes/tutor-notes.component.spec.ts @@ -0,0 +1,245 @@ +import {beforeEach, describe, expect, it, vi} from 'vitest'; +import {NO_ERRORS_SCHEMA} from '@angular/core'; +import {ComponentFixture, TestBed} from '@angular/core/testing'; +import {MatButtonModule} from '@angular/material/button'; +import {MatChipSelectionChange} from '@angular/material/chips'; +import {EMPTY, of, throwError} from 'rxjs'; +import {Task, UserService} from 'src/app/api/models/doubtfire-model'; +import {TutorNote} from 'src/app/api/models/tutor-note'; +import {TutorNoteService} from 'src/app/api/services/tutor-note.service'; +import {EmptyStateComponent} from 'src/app/common/empty-state/empty-state.component'; +import {ConfirmationModalService} from 'src/app/common/modals/confirmation-modal/confirmation-modal.service'; +import {HumanizedDatePipe} from 'src/app/common/pipes/humanized-date.pipe'; +import {LocalizedDatePipe} from 'src/app/common/pipes/localized-date.pipe'; +import {MarkedPipe} from 'src/app/common/pipes/marked.pipe'; +import {AlertService} from 'src/app/common/services/alert.service'; +import {TutorNotesComponent} from './tutor-notes.component'; + +const emptyProvider = {}; +const tutorNoteServiceStub = { + loadTutorNotes: () => EMPTY, + updateTutorNoteReplies: () => undefined, +}; + +describe('TutorNotesComponent', () => { + let component: TutorNotesComponent; + let fixture: ComponentFixture; + let note: TutorNote; + + beforeEach(async () => { + await TestBed.configureTestingModule({ + declarations: [TutorNotesComponent, HumanizedDatePipe, LocalizedDatePipe, MarkedPipe], + providers: [ + {provide: UserService, useValue: emptyProvider}, + {provide: TutorNoteService, useValue: tutorNoteServiceStub}, + {provide: AlertService, useValue: emptyProvider}, + {provide: ConfirmationModalService, useValue: emptyProvider}, + ], + schemas: [NO_ERRORS_SCHEMA], + }).compileComponents(); + }); + + beforeEach(() => { + fixture = TestBed.createComponent(TutorNotesComponent); + component = fixture.componentInstance; + note = { + id: 3, + replyToId: null, + note: 'a tutor note', + user: {}, + authorIsMe: true, + noteIsForMe: true, + readByUnitRole: false, + } as TutorNote; + component.unitRole = {tutorNotesCache: {currentValues: [note]}} as never; + + fixture.detectChanges(); + component.loadingTutorNotes = false; + fixture.detectChanges(); + }); + + it('should create', () => { + expect(component).toBeTruthy(); + }); + + function card(): HTMLElement { + return fixture.nativeElement.querySelector('.note-card') as HTMLElement; + } + + function editButton(): HTMLButtonElement { + return card().querySelector('.note-actions button') as HTMLButtonElement; + } + + it('renders the note actions instead of gating them behind a pointer flag', () => { + const actions = card().querySelector('.note-actions') as HTMLElement; + + expect(actions).toBeTruthy(); + expect(actions.hasAttribute('hidden')).toBe(false); + }); + + it('builds the actions out of real buttons rather than bare icons', () => { + const buttons = card().querySelectorAll('.note-actions button'); + + expect(buttons.length).toBe(3); + expect(editButton().getAttribute('aria-label')).toBe('Edit this note'); + }); + + it('does not meet the reveal condition while nothing in the note has focus', () => { + expect(card().matches(':focus-within')).toBe(false); + }); + + it('meets the reveal condition once the keyboard reaches the actions', () => { + editButton().focus(); + + expect(document.activeElement).toBe(editButton()); + expect(card().matches(':focus-within')).toBe(true); + }); + + it('opens the editor through the button the pointer uses, not through injected state', () => { + const button = editButton(); + button.focus(); + button.click(); + fixture.detectChanges(); + + expect(component.editingNote).toBe(note); + expect(card().querySelector('textarea')).toBeTruthy(); + }); + + it('keeps Mark as read outside the note actions', () => { + const markAsRead = Array.from(card().querySelectorAll('button')).find( + (button) => button.textContent.trim() === 'Mark as read', + ); + + expect(markAsRead).toBeDefined(); + expect(markAsRead.closest('.note-actions')).toBeNull(); + }); +}); + +describe('TutorNotesComponent states', () => { + let component: TutorNotesComponent; + let fixture: ComponentFixture; + let loadTutorNotes: ReturnType; + let addNote: ReturnType; + + const otherTaskNote = { + id: 4, + replyToId: null, + note: 'a note on another task', + user: {}, + authorIsMe: false, + taskDefinition: {abbreviation: 'T2', name: 'Second task'}, + } as TutorNote; + + beforeEach(async () => { + loadTutorNotes = vi.fn(() => of([])); + addNote = vi.fn(() => EMPTY); + + await TestBed.configureTestingModule({ + declarations: [TutorNotesComponent, HumanizedDatePipe, LocalizedDatePipe, MarkedPipe], + // The real button, so disabledInteractive behaves as it does in the app. + imports: [EmptyStateComponent, MatButtonModule], + providers: [ + {provide: UserService, useValue: emptyProvider}, + { + provide: TutorNoteService, + useValue: {loadTutorNotes, addNote, updateTutorNoteReplies: () => undefined}, + }, + {provide: AlertService, useValue: emptyProvider}, + {provide: ConfirmationModalService, useValue: emptyProvider}, + ], + schemas: [NO_ERRORS_SCHEMA], + }).compileComponents(); + }); + + // Opened from a task, the list starts filtered to that task. + function render(notes: TutorNote[], task?: Task): void { + fixture = TestBed.createComponent(TutorNotesComponent); + component = fixture.componentInstance; + component.task = task; + component.unitRole = {tutorNotesCache: {currentValues: notes}} as never; + fixture.detectChanges(); + } + + function text(): string { + return fixture.nativeElement.textContent.replace(/\s+/g, ' '); + } + + function filters(): HTMLElement | null { + return fixture.nativeElement.querySelector('mat-chip-listbox'); + } + + function userChange(selected: boolean): MatChipSelectionChange { + return {isUserInput: true, selected} as MatChipSelectionChange; + } + + it('says there are no moderation notes yet, without filters that would do nothing', () => { + render([]); + + expect(text()).toContain('No moderation notes yet'); + expect(filters()).toBeNull(); + expect(fixture.nativeElement.querySelector('.note-card')).toBeNull(); + }); + + it('explains an empty filter and offers the filters to widen it', () => { + render([otherTaskNote], {definition: {abbreviation: 'T1'}} as Task); + + expect(text()).toContain('No notes for the selected tasks'); + expect(text()).not.toContain('No moderation notes yet'); + expect(filters()).not.toBeNull(); + + component.onFilterChange('all', userChange(true)); + fixture.detectChanges(); + + expect(text()).not.toContain('No notes for the selected tasks'); + expect(fixture.nativeElement.querySelectorAll('.note-card').length).toBe(1); + }); + + it('only follows filter changes the user made', () => { + render([otherTaskNote], {definition: {abbreviation: 'T1'}} as Task); + + component.onFilterChange('T1', {isUserInput: false, selected: false} as MatChipSelectionChange); + expect(component.selectedTaskDefinitions.get('T1')).toBe(true); + + component.onFilterChange('T1', userChange(false)); + expect(component.selectedTaskDefinitions.get('T1')).toBe(false); + }); + + it('shows an error state with Try again when the notes fail to load', () => { + loadTutorNotes.mockReturnValueOnce(throwError(() => new Error('offline'))); + render([]); + + expect(text()).toContain('The notes did not load'); + + const tryAgain = Array.from( + fixture.nativeElement.querySelectorAll('button'), + ).find((button) => button.textContent.trim() === 'Try again'); + tryAgain.click(); + fixture.detectChanges(); + + expect(loadTutorNotes).toHaveBeenCalledTimes(2); + expect(text()).not.toContain('The notes did not load'); + expect(text()).toContain('No moderation notes yet'); + }); + + it('puts Submit outside the text field, focusable but inactive until there is text', () => { + render([]); + const submit = Array.from( + fixture.nativeElement.querySelectorAll('button'), + ).find( + (button) => + (button.querySelector('.mdc-button__label') ?? button).textContent.trim() === 'Submit', + ); + + expect(submit.closest('mat-form-field')).toBeNull(); + expect(submit.querySelector('mat-icon')?.textContent.trim()).toBe('save'); + expect(submit.hasAttribute('disabled')).toBe(false); + expect(submit.getAttribute('aria-disabled')).toBe('true'); + + submit.click(); + expect(addNote).not.toHaveBeenCalled(); + + component.noteText = 'Can we talk about this mark?'; + fixture.detectChanges(); + expect(submit.getAttribute('aria-disabled')).toBeNull(); + }); +}); diff --git a/src/app/projects/states/tutor-notes/tutor-notes.component.ts b/src/app/projects/states/tutor-notes/tutor-notes.component.ts index 94f258a5ae..d0c6cdf668 100644 --- a/src/app/projects/states/tutor-notes/tutor-notes.component.ts +++ b/src/app/projects/states/tutor-notes/tutor-notes.component.ts @@ -6,6 +6,7 @@ import { OnInit, ViewChild, } from '@angular/core'; +import {MatChipSelectionChange} from '@angular/material/chips'; import {Task, UnitRole, UserService} from 'src/app/api/models/doubtfire-model'; import {TutorNote} from 'src/app/api/models/tutor-note'; import {TutorNoteService} from 'src/app/api/services/tutor-note.service'; @@ -27,6 +28,8 @@ export class TutorNotesComponent implements OnInit { @Input() task: Task; loadingTutorNotes: boolean = true; + /** The last load failed, so the list offers a retry instead of saying there are none. */ + loadError = false; noteText: string = ''; @@ -35,8 +38,6 @@ export class TutorNotesComponent implements OnInit { replyingToNote?: TutorNote; - hoveredNoteId: number | null = null; - constructor( private userService: UserService, private tutorNoteService: TutorNoteService, @@ -48,12 +49,7 @@ export class TutorNotesComponent implements OnInit { this.unitRole = this.task.tutor; } - this.loadingTutorNotes = true; - this.tutorNoteService.loadTutorNotes(this.unitRole).subscribe((_notes) => { - this.loadingTutorNotes = false; - this.tutorNoteService.updateTutorNoteReplies(this.unitRole?.tutorNotesCache.currentValues); - this.scrollDown(); - }); + this.loadNotes(); if (this.task) { this.selectedTaskDefinitions.set(this.task.definition.abbreviation, true); } else { @@ -61,6 +57,26 @@ export class TutorNotesComponent implements OnInit { } } + public loadNotes(): void { + this.loadingTutorNotes = true; + this.loadError = false; + this.tutorNoteService.loadTutorNotes(this.unitRole).subscribe({ + next: (_notes) => { + this.loadingTutorNotes = false; + this.tutorNoteService.updateTutorNoteReplies(this.unitRole?.tutorNotesCache.currentValues); + this.scrollDown(); + }, + error: () => { + this.loadingTutorNotes = false; + this.loadError = true; + }, + }); + } + + public get allNotes(): readonly TutorNote[] { + return this.unitRole?.tutorNotesCache?.currentValues ?? []; + } + scrollToComment(commentID: number) { document.querySelector(`#comment-${commentID}`).scrollIntoView(); } @@ -84,7 +100,7 @@ export class TutorNotesComponent implements OnInit { .addNote(this.unitRole, noteText, this.task, this.replyingToNote) .subscribe({ next: (_note) => { - this.alertService.success('Succesfully submitted note', 4000); + this.alertService.success('Successfully submitted note', 4000); this.scrollDown(); this.replyingToNote = null; this.tutorNoteService.updateTutorNoteReplies( @@ -106,7 +122,7 @@ export class TutorNotesComponent implements OnInit { this.tutorNoteService.updateNote(this.unitRole, this.editingNote, noteText).subscribe({ next: (_note) => { - this.alertService.success('Succesfully updated note', 4000); + this.alertService.success('Successfully updated note', 4000); this.editingNote = null; this.editingNoteText = ''; }, @@ -199,12 +215,13 @@ export class TutorNotesComponent implements OnInit { ); } - toggleSelection(option: string) { - if (this.selectedTaskDefinitions.get(option)) { - this.selectedTaskDefinitions.set(option, false); - } else { - this.selectedTaskDefinitions.set(option, true); + // Follows the chip's own selection event rather than a click, so a keyboard toggle + // filters the list too. Programmatic changes from the [selected] binding are ignored. + onFilterChange(option: string, change: MatChipSelectionChange) { + if (!change.isUserInput) { + return; } + this.selectedTaskDefinitions.set(option, change.selected); } public get taskDefinitionFilters() { @@ -217,11 +234,9 @@ export class TutorNotesComponent implements OnInit { return Array.from(new Set(abbrs)); } - openProject(event: Event, note: TutorNote) { - event.stopPropagation(); - const link = document.createElement('a'); - link.href = `/projects/${note.project.id}/dashboard/${note.taskDefinition.abbreviation}?tutor=true`; - link.target = '_blank'; - link.click(); + /** The student's task, opened as staff. A real link, so it works from the keyboard. */ + projectLink(note: TutorNote): string { + const abbreviation = note.taskDefinition?.abbreviation ?? ''; + return `/projects/${note.project.id}/dashboard/${abbreviation}?tutor=true`; } } diff --git a/src/app/sessions/states/sign-in/sign-in.component.html b/src/app/sessions/states/sign-in/sign-in.component.html index 2c57ca1db0..31c8c1a351 100644 --- a/src/app/sessions/states/sign-in/sign-in.component.html +++ b/src/app/sessions/states/sign-in/sign-in.component.html @@ -1,15 +1,15 @@ -
+
-
+
@if (!isLoading) { @if (!isLoading) { -
+
-
- Homepage Logo +
+ OnTrack logo

{{ externalName.value }}

-

+

Welcome to {{ externalName.value }}

@if (showCredentials) { Username - + } @if (showCredentials) { Password
+
} diff --git a/src/app/tasks/modals/grade-task-modal/grade-task-modal.accessibility.spec.ts b/src/app/tasks/modals/grade-task-modal/grade-task-modal.accessibility.spec.ts new file mode 100644 index 0000000000..6fa4cec30c --- /dev/null +++ b/src/app/tasks/modals/grade-task-modal/grade-task-modal.accessibility.spec.ts @@ -0,0 +1,83 @@ +import {NO_ERRORS_SCHEMA} from '@angular/core'; +import {TestBed} from '@angular/core/testing'; +import {FormsModule} from '@angular/forms'; +import {MatButtonModule} from '@angular/material/button'; +import {MatButtonToggleModule} from '@angular/material/button-toggle'; +import {MatCardModule} from '@angular/material/card'; +import {MatDialog, MatDialogModule} from '@angular/material/dialog'; +import {MatSliderModule} from '@angular/material/slider'; +import {MatTooltipModule} from '@angular/material/tooltip'; +import {GradeService} from 'src/app/common/services/grade.service'; +import {expectAccessible} from 'src/app/common/testing/accessibility'; +import {GradeTaskModalComponent} from './grade-task-modal.component'; + +describe('GradeTaskModalComponent rendered accessibility', () => { + beforeEach(async () => { + await TestBed.configureTestingModule({ + declarations: [GradeTaskModalComponent], + imports: [ + FormsModule, + MatButtonModule, + MatButtonToggleModule, + MatCardModule, + MatDialogModule, + MatSliderModule, + MatTooltipModule, + ], + providers: [ + { + provide: GradeService, + useValue: { + allGradeValuesFor: () => [1, 2], + gradeLabel: (grade: number) => (grade === 1 ? 'Pass' : 'Credit'), + }, + }, + ], + schemas: [NO_ERRORS_SCHEMA], + }).compileComponents(); + }); + + afterEach(() => TestBed.inject(MatDialog).closeAll()); + + it('names the real dialog, grade group and quality slider without invalid ID references', async () => { + const ref = TestBed.inject(MatDialog).open(GradeTaskModalComponent, { + data: { + task: { + grade: 1, + qualityPts: 2, + unit: {}, + definition: {isGraded: true, maxQualityPts: 5, abbreviation: 'DEMO'}, + }, + }, + }); + TestBed.tick(); + await Promise.resolve(); + TestBed.tick(); + const dialog = document.querySelector('mat-dialog-container')!; + await expectAccessible(dialog as HTMLElement); + const titleId = dialog.getAttribute('aria-labelledby')!; + expect(document.getElementById(titleId)?.textContent).toContain('Assess task quality'); + expect(dialog.querySelector('[role="radiogroup"]')?.getAttribute('aria-label')).toBe( + 'Task grade', + ); + const options = dialog.querySelectorAll('mat-button-toggle button'); + expect([...options].map((option) => option.getAttribute('aria-label'))).toEqual([ + 'Mark task as Pass', + 'Mark task as Credit', + ]); + for (const described of dialog.querySelectorAll('[aria-describedby]')) { + for (const id of described.getAttribute('aria-describedby')!.split(/\s+/)) { + expect(document.getElementById(id)).not.toBeNull(); + } + } + const slider = dialog.querySelector('input[matSliderThumb]')!; + expect(slider.getAttribute('aria-label')).toBe('Task quality rating'); + expect(slider.getAttribute('aria-valuetext')).toBe('2 out of 5'); + ref.componentInstance.updateRating(0); + TestBed.tick(); + await Promise.resolve(); + TestBed.tick(); + expect(slider.getAttribute('aria-valuetext')).toBe('0 out of 5'); + expect(ref.componentInstance.isValid()).toBeTruthy(); + }); +}); diff --git a/src/app/tasks/modals/grade-task-modal/grade-task-modal.component.html b/src/app/tasks/modals/grade-task-modal/grade-task-modal.component.html index 2b8072a9dd..2c34fd250b 100644 --- a/src/app/tasks/modals/grade-task-modal/grade-task-modal.component.html +++ b/src/app/tasks/modals/grade-task-modal/grade-task-modal.component.html @@ -1,75 +1,80 @@ - - - Assess Task Quality - - - - @if (task.definition.isGraded) { -
-

- Please provide a grade for task: - - {{ task.definition.abbreviation }} - -

-
- - @for (idx of gradeValues; track idx) { - - - - } - -
-
- } - - @if (task.definition.maxQualityPts > 0) { -
-

- Please provide a quality rating for task: - +

Assess task quality

+ + +

+ + {{ task.definition.abbreviation }} + + {{ task.definition.name }} +

+ + @if (task.definition.isGraded) { +
+

Grade

+ + @for (idx of gradeValues; track idx) { + - {{ task.definition.abbreviation }} - -

- - - -
-

Rating: {{ rating }} / {{ totalRating }}

-
+ + + } + +
+ } + + @if (task.definition.maxQualityPts > 0) { +
+
+

Quality rating

+
- } - - -
- - -
- - + + +
+ } + + + + + + diff --git a/src/app/tasks/modals/grade-task-modal/grade-task-modal.component.ts b/src/app/tasks/modals/grade-task-modal/grade-task-modal.component.ts index b6a8adb053..0c5dc3709b 100644 --- a/src/app/tasks/modals/grade-task-modal/grade-task-modal.component.ts +++ b/src/app/tasks/modals/grade-task-modal/grade-task-modal.component.ts @@ -22,6 +22,8 @@ export class GradeTaskModalComponent implements OnInit { // Grade Select selectedGrade: number; + readonly formatQualityRating = (value: number): string => `${value} out of ${this.totalRating}`; + constructor( public dialogRef: MatDialogRef, @Inject(MAT_DIALOG_DATA) public dialogData: {task: Task}, diff --git a/src/app/tasks/task-comment-composer/task-comment-composer.accessibility.spec.ts b/src/app/tasks/task-comment-composer/task-comment-composer.accessibility.spec.ts new file mode 100644 index 0000000000..f76037d3d0 --- /dev/null +++ b/src/app/tasks/task-comment-composer/task-comment-composer.accessibility.spec.ts @@ -0,0 +1,109 @@ +import {EmojiSearch} from '@ctrl/ngx-emoji-mart'; +import {afterEach, beforeEach, describe, expect, it, vi} from 'vitest'; +import {CommonModule} from '@angular/common'; +import {NO_ERRORS_SCHEMA} from '@angular/core'; +import {ComponentFixture, TestBed} from '@angular/core/testing'; +import {MatButtonModule} from '@angular/material/button'; +import {MatDialog} from '@angular/material/dialog'; +import {provideNoopAnimations} from '@angular/platform-browser/animations'; +import {of} from 'rxjs'; +import {TaskCommentService, UserService} from 'src/app/api/models/doubtfire-model'; +import {AlertService} from 'src/app/common/services/alert.service'; +import {EmojiService} from 'src/app/common/services/emoji.service'; +import {expectAccessible} from 'src/app/common/testing/accessibility'; +import {TaskCommentsViewerComponent} from '../task-comments-viewer/task-comments-viewer.component'; +import {TaskCommentComposerComponent} from './task-comment-composer.component'; + +describe('Shared task comment controls', () => { + let fixture: ComponentFixture; + + beforeEach(async () => { + for (const name of ['localStorage', 'sessionStorage']) { + vi.stubGlobal(name, {getItem: () => null, setItem: vi.fn(), removeItem: vi.fn()}); + } + await TestBed.configureTestingModule({ + imports: [CommonModule, MatButtonModule], + declarations: [TaskCommentComposerComponent], + providers: [ + provideNoopAnimations(), + {provide: MatDialog, useValue: {}}, + {provide: EmojiSearch, useValue: {}}, + {provide: EmojiService, useValue: {}}, + {provide: TaskCommentsViewerComponent, useValue: {scrollDown: vi.fn()}}, + {provide: AlertService, useValue: {}}, + { + provide: TaskCommentService, + useValue: { + attachmentPolicy: () => + of({ + version: 1, + max_bytes_exclusive: 10_000_000, + max_selection_count: 5, + categories: [{id: 'pdf', name: 'PDF', extensions: ['pdf'], preview: 'pdf'}], + }), + }, + }, + {provide: UserService, useValue: {currentUser: {id: 1}}}, + ], + schemas: [NO_ERRORS_SCHEMA], + }).compileComponents(); + fixture = TestBed.createComponent(TaskCommentComposerComponent); + fixture.componentRef.setInput('task', {id: 123, unit: {currentUserIsStaff: false}}); + fixture.componentRef.setInput('sharedData', {originalComment: null, editingComment: null}); + fixture.detectChanges(); + }); + + afterEach(() => vi.unstubAllGlobals()); + + it('passes automated accessibility checks for student and staff feedback controls', async () => { + await fixture.whenStable(); + await expectAccessible(fixture.nativeElement); + fixture.componentRef.setInput('task', {id: 456, unit: {currentUserIsStaff: true}}); + fixture.detectChanges(); + await fixture.whenStable(); + await expectAccessible(fixture.nativeElement); + }); + + it('exposes a named multiline textbox and removes it from focus while recording', () => { + const editor = fixture.nativeElement.querySelector('[role="textbox"]') as HTMLElement; + expect(editor.getAttribute('aria-label')).toBe('Task comment'); + expect(editor.getAttribute('aria-multiline')).toBe('true'); + expect(editor.tabIndex).toBe(0); + editor.focus(); + expect(document.activeElement).toBe(editor); + fixture.componentInstance.recording = true; + fixture.detectChanges(); + expect(editor.hidden).toBe(true); + expect(editor.tabIndex).toBe(-1); + expect(editor.getAttribute('aria-disabled')).toBe('true'); + }); + + it('uses a focusable native button to open and close emoji choices once per activation', () => { + const button = fixture.nativeElement.querySelector( + 'button[aria-label="Choose an emoji"]', + ) as HTMLButtonElement; + expect(button.type).toBe('button'); + expect(button.tabIndex).toBe(0); + button.click(); + fixture.detectChanges(); + expect(button.getAttribute('aria-expanded')).toBe('true'); + button.click(); + fixture.detectChanges(); + expect(button.getAttribute('aria-expanded')).toBe('false'); + }); + + it('keeps the staff feedback template action hidden from students', () => { + const button = fixture.nativeElement.querySelector( + 'button[aria-label="Choose feedback templates"]', + ) as HTMLButtonElement; + expect(button.closest('[hidden]')).toBeTruthy(); + fixture.componentRef.setInput('task', {id: 456, unit: {currentUserIsStaff: true}}); + fixture.detectChanges(); + expect(button.closest('[hidden]')).toBeNull(); + const open = vi.spyOn(fixture.componentInstance, 'showFeedbackPicker'); + button.click(); + fixture.detectChanges(); + expect(open).toHaveBeenCalledOnce(); + expect(button.getAttribute('aria-expanded')).toBe('true'); + }); +}); diff --git a/src/app/tasks/task-comments-viewer/comment-bubble-action/comment-bubble-action.component.html b/src/app/tasks/task-comments-viewer/comment-bubble-action/comment-bubble-action.component.html index f3b9b5c748..a0703f57bb 100644 --- a/src/app/tasks/task-comments-viewer/comment-bubble-action/comment-bubble-action.component.html +++ b/src/app/tasks/task-comments-viewer/comment-bubble-action/comment-bubble-action.component.html @@ -3,33 +3,36 @@ aria-label="React to this comment"> tag_faces --> - - reply - reply + + +
diff --git a/src/app/tasks/task-comments-viewer/pdf-image-comment/pdf-image-comment.accessibility.spec.ts b/src/app/tasks/task-comments-viewer/pdf-image-comment/pdf-image-comment.accessibility.spec.ts new file mode 100644 index 0000000000..588ffd6f45 --- /dev/null +++ b/src/app/tasks/task-comments-viewer/pdf-image-comment/pdf-image-comment.accessibility.spec.ts @@ -0,0 +1,86 @@ +import {beforeEach, describe, expect, it, vi} from 'vitest'; +import {ComponentFixture, TestBed} from '@angular/core/testing'; +import {MatIconModule} from '@angular/material/icon'; +import {TaskComment} from 'src/app/api/models/doubtfire-model'; +import {FileDownloaderService} from 'src/app/common/file-downloader/file-downloader.service'; +import {CommentsModalService} from 'src/app/common/modals/comments-modal/comments-modal.service'; +import {SafePipe} from 'src/app/common/pipes/safe.pipe'; +import {AlertService} from 'src/app/common/services/alert.service'; +import {SentAttachmentCardComponent} from '../sent-attachment-card/sent-attachment-card.component'; +import {PdfImageCommentComponent} from './pdf-image-comment.component'; + +describe('PDF and image attachment keyboard controls', () => { + let fixture: ComponentFixture; + let downloader: {downloadBlob: ReturnType; releaseBlob: ReturnType}; + let modal: {show: ReturnType}; + let alerts: {error: ReturnType}; + + beforeEach(async () => { + downloader = { + downloadBlob: vi.fn((_url, success) => success('blob:attachment', undefined)), + releaseBlob: vi.fn(), + }; + modal = {show: vi.fn()}; + alerts = {error: vi.fn()}; + await TestBed.configureTestingModule({ + declarations: [PdfImageCommentComponent, SafePipe, SentAttachmentCardComponent], + imports: [MatIconModule], + providers: [ + {provide: FileDownloaderService, useValue: downloader}, + {provide: CommentsModalService, useValue: modal}, + {provide: AlertService, useValue: alerts}, + ], + }).compileComponents(); + fixture = TestBed.createComponent(PdfImageCommentComponent); + }); + + // A PDF opens its authorised URL straight away and the modal owns the loading state. + // An image is fetched on load, so the preview button opens the fetched copy. + it.each(['pdf', 'image'])('opens the %s preview from a named native button', (commentType) => { + const comment = {commentType, attachmentUrl: '/attachment'} as TaskComment; + fixture.componentInstance.comment = comment; + fixture.detectChanges(); + + const button: HTMLButtonElement = fixture.nativeElement.querySelector('button'); + expect(button).not.toBeNull(); + expect(button.type).toBe('button'); + expect(button.tabIndex).toBe(0); + expect(button.getAttribute('aria-label')).toBe( + commentType === 'pdf' ? 'Preview PDF attachment: PDF attachment' : 'Preview image attachment', + ); + button.focus(); + expect(document.activeElement).toBe(button); + const enter = new KeyboardEvent('keydown', {key: 'Enter', bubbles: true, cancelable: true}); + button.dispatchEvent(enter); + expect(enter.defaultPrevented).toBe(false); + // jsdom does not synthesize the native button activation from Enter. + button.click(); + + expect(modal.show).toHaveBeenCalledExactlyOnceWith( + commentType === 'pdf' ? '/attachment' : 'blob:attachment', + comment, + ); + expect(downloader.downloadBlob).toHaveBeenCalledTimes(commentType === 'pdf' ? 0 : 1); + expect(alerts.error).not.toHaveBeenCalled(); + if (commentType === 'image') { + expect(button.querySelector('img').getAttribute('alt')).toBe('Image attachment preview'); + } + }); + + it('reports a failed attachment download without opening an empty preview', () => { + downloader.downloadBlob.mockImplementation((_url, _success, failure) => failure('offline')); + fixture.componentInstance.comment = { + commentType: 'image', + attachmentUrl: '/attachment', + } as TaskComment; + fixture.detectChanges(); + + fixture.nativeElement.querySelector('button').click(); + + expect(modal.show).not.toHaveBeenCalled(); + expect(alerts.error).toHaveBeenCalledWith( + 'Unable to load this image attachment. Please try again.', + 6000, + ); + }); +}); diff --git a/src/app/tasks/task-comments-viewer/scorm-comment/scorm-comment.component.html b/src/app/tasks/task-comments-viewer/scorm-comment/scorm-comment.component.html index c9c39db6a6..f3376e4b05 100644 --- a/src/app/tasks/task-comments-viewer/scorm-comment/scorm-comment.component.html +++ b/src/app/tasks/task-comments-viewer/scorm-comment/scorm-comment.component.html @@ -16,6 +16,7 @@ } + @if ( + comment.commentType === 'document' || comment.commentType === 'spreadsheet' + ) { +
+ {{ comment.attachmentFileName || 'Attachment' }} + {{ + comment.commentType === 'spreadsheet' ? 'Spreadsheet' : 'Document' + }} + · {{ comment.attachmentByteSize }} bytes · Download only + +

{{ comment.text }}

+
+ } @switch (comment.commentType) { @case ('extension') {
@@ -273,13 +301,20 @@ >
} + + @case ('document') { + + } }
@if (comment.isBubbleComment) {
@@ -304,10 +339,12 @@
} +
-