diff --git a/src/app/api/fixtures/peer-progress-indicator.fixtures.spec.ts b/src/app/api/fixtures/peer-progress-indicator.fixtures.spec.ts deleted file mode 100644 index 1e6f5cb0d3..0000000000 --- a/src/app/api/fixtures/peer-progress-indicator.fixtures.spec.ts +++ /dev/null @@ -1,178 +0,0 @@ -import {describe, expect, it} from 'vitest'; -import {PeerProgressIndicator} from '../models/peer-progress-indicator'; -import { - FIXTURE_DISABLED, - FIXTURE_MALFORMED, - FIXTURE_NORMAL, - FIXTURE_STALE, - FIXTURE_SUPPRESSED, - FIXTURE_UNAVAILABLE, - FIXTURE_ZERO_PERCENT, -} from './peer-progress-indicator.fixtures'; - -const assertContractShape = (fixture: PeerProgressIndicator) => { - expect(typeof fixture.taskDefinitionId).toBe('number'); - expect(typeof fixture.unitId).toBe('number'); - expect(fixture.targetGrade === null || typeof fixture.targetGrade === 'number').toBe(true); - - // submittedPercentage can be number or null - expect( - fixture.submittedPercentage === null || typeof fixture.submittedPercentage === 'number', - ).toBe(true); - expect( - fixture.completedPercentage === null || typeof fixture.completedPercentage === 'number', - ).toBe(true); - expect(typeof fixture.distributionAvailable).toBe('boolean'); - expect(Array.isArray(fixture.statusDistribution)).toBe(true); - expect(typeof fixture.isUserEnabled).toBe('boolean'); - - expect(typeof fixture.isSuppressed).toBe('boolean'); - expect(typeof fixture.isStale).toBe('boolean'); - expect(typeof fixture.isFeatureEnabled).toBe('boolean'); - - expect(fixture.lastUpdatedAt === null || typeof fixture.lastUpdatedAt === 'string').toBe(true); - expect(typeof fixture.unavailableMessage).toBe('string'); - expect(fixture.unavailableReason === null || typeof fixture.unavailableReason === 'string').toBe( - true, - ); - expect( - fixture.distributionUnavailableReason === null || - typeof fixture.distributionUnavailableReason === 'string', - ).toBe(true); -}; - -describe('PeerProgressIndicator Fixture Regression Tests', () => { - // Contract Shape Tests - it('NORMAL fixture matches the contract shape', () => { - assertContractShape(FIXTURE_NORMAL); - }); - - it('ZERO_PERCENT fixture matches the contract shape', () => { - assertContractShape(FIXTURE_ZERO_PERCENT); - }); - - it('SUPPRESSED fixture matches the contract shape', () => { - assertContractShape(FIXTURE_SUPPRESSED); - }); - - it('UNAVAILABLE fixture matches the contract shape', () => { - assertContractShape(FIXTURE_UNAVAILABLE); - }); - - it('DISABLED fixture matches the contract shape', () => { - assertContractShape(FIXTURE_DISABLED); - }); - - it('STALE fixture matches the contract shape', () => { - assertContractShape(FIXTURE_STALE); - }); - - // Safe-State Behaviour Tests - it('NORMAL fixture has a valid percentage', () => { - expect(FIXTURE_NORMAL.submittedPercentage).toBeGreaterThan(0); - expect(FIXTURE_NORMAL.isSuppressed).toBe(false); - expect(FIXTURE_NORMAL.isFeatureEnabled).toBe(true); - }); - - it('NORMAL fixture contains the complete, 10-point-quantised canonical status vector', () => { - expect(FIXTURE_NORMAL.statusDistribution).toHaveLength(15); - expect(new Set(FIXTURE_NORMAL.statusDistribution.map((entry) => entry.status)).size).toBe(15); - expect(FIXTURE_NORMAL.statusDistribution.every((entry) => entry.percentage % 10 === 0)).toBe( - true, - ); - expect(FIXTURE_NORMAL.statusDistribution).toContainEqual({ - status: 'fix_and_resubmit', - percentage: 10, - }); - expect(FIXTURE_NORMAL.statusDistribution).toContainEqual({ - status: 'redo', - percentage: 10, - }); - }); - - it('ZERO_PERCENT fixture keeps a displayed 0% distinct from unavailable data', () => { - expect(FIXTURE_ZERO_PERCENT.submittedPercentage).toBe(0); - expect(FIXTURE_ZERO_PERCENT.isSuppressed).toBe(false); - }); - - it('SUPPRESSED fixture hides percentage and uses safe wording', () => { - expect(FIXTURE_SUPPRESSED.submittedPercentage).toBeNull(); - expect(FIXTURE_SUPPRESSED.isSuppressed).toBe(true); - expect(FIXTURE_SUPPRESSED.unavailableMessage).toContain('Not enough students'); - }); - - it('UNAVAILABLE fixture hides percentage but is not suppressed', () => { - expect(FIXTURE_UNAVAILABLE.submittedPercentage).toBeNull(); - expect(FIXTURE_UNAVAILABLE.isSuppressed).toBe(false); - expect(FIXTURE_UNAVAILABLE.unavailableMessage).toContain('Progress unavailable'); - }); - - it('DISABLED fixture hides percentage and marks feature disabled', () => { - expect(FIXTURE_DISABLED.submittedPercentage).toBeNull(); - expect(FIXTURE_DISABLED.isFeatureEnabled).toBe(false); - }); - - it('STALE fixture marks data as stale', () => { - expect(FIXTURE_STALE.isStale).toBe(true); - }); - - // Privacy Tests - it('No fixture leaks peer identities or raw cohort counts', () => { - const fixtures = [ - FIXTURE_NORMAL, - FIXTURE_ZERO_PERCENT, - FIXTURE_SUPPRESSED, - FIXTURE_UNAVAILABLE, - FIXTURE_DISABLED, - FIXTURE_STALE, - ]; - - fixtures.forEach((f) => { - expect(f).not.toHaveProperty('peerName'); - expect(f).not.toHaveProperty('studentId'); - expect(f).not.toHaveProperty('cohortCount'); - expect(f).not.toHaveProperty('marks'); - expect(f).not.toHaveProperty('feedback'); - expect(f.statusDistribution).not.toContainEqual( - expect.objectContaining({studentId: expect.anything()}), - ); - }); - }); - - // Malformed Safe-Failure Tests - it('MALFORMED fixture contains invalid values for safe-failure testing', () => { - const malformed = FIXTURE_MALFORMED as Partial; - - // invalid numeric values - expect(malformed.submittedPercentage).toBeNaN(); - expect(Number.isFinite(malformed.submittedPercentage as number)).toBe(false); - expect(typeof malformed.distributionAvailable).not.toBe('boolean'); - - // invalid ranges - // expect((malformed.submittedPercentage as number) < 0).toBe(true); - // expect((malformed.submittedPercentage as number) > 100).toBe(true); - - // invalid timestamp - expect(isNaN(Date.parse(malformed.lastUpdatedAt as string))).toBe(true); - - // invalid IDs - expect(typeof malformed.taskDefinitionId).not.toBe('number'); - expect(typeof malformed.unitId).not.toBe('number'); - - // invalid target grade - expect(typeof malformed.targetGrade).not.toBe('number'); - }); - - it('MALFORMED fixture must not allow impossible state combinations', () => { - const malformed = FIXTURE_MALFORMED as Partial; - - // suppressed must be boolean - expect(typeof malformed.isSuppressed).not.toBe('boolean'); - - // stale must be boolean - expect(typeof malformed.isStale).not.toBe('boolean'); - - // featureEnabled must be boolean - expect(typeof malformed.isFeatureEnabled).not.toBe('boolean'); - }); -}); diff --git a/src/app/api/fixtures/peer-progress-indicator.fixtures.ts b/src/app/api/fixtures/peer-progress-indicator.fixtures.ts deleted file mode 100644 index 17c525f52e..0000000000 --- a/src/app/api/fixtures/peer-progress-indicator.fixtures.ts +++ /dev/null @@ -1,160 +0,0 @@ -import {PeerProgressIndicator} from '../models/peer-progress-indicator'; - -export const FIXTURE_NORMAL: PeerProgressIndicator = { - taskDefinitionId: 12, - unitId: 5, - targetGrade: 2, - submittedPercentage: 60, - completedPercentage: 10, - distributionAvailable: true, - statusDistribution: [ - {status: 'not_started', percentage: 20}, - {status: 'feedback_exceeded', percentage: 0}, - {status: 'redo', percentage: 10}, - {status: 'need_help', percentage: 0}, - {status: 'working_on_it', percentage: 20}, - {status: 'fix_and_resubmit', percentage: 10}, - {status: 'ready_for_feedback', percentage: 20}, - {status: 'discuss', percentage: 0}, - {status: 'demonstrate', percentage: 0}, - {status: 'complete', percentage: 10}, - {status: 'fail', percentage: 10}, - {status: 'time_exceeded', percentage: 0}, - {status: 'assess_in_portfolio', percentage: 0}, - {status: 'attention_required', percentage: 0}, - {status: 'rediscuss', percentage: 0}, - ], - isUserEnabled: true, - isSuppressed: false, - isStale: false, - isFeatureEnabled: true, - lastUpdatedAt: '2026-08-14T03:15:00Z', - unavailableMessage: '', - unavailableReason: null, - distributionUnavailableReason: null, -}; - -export const FIXTURE_ZERO_PERCENT: PeerProgressIndicator = { - taskDefinitionId: 12, - unitId: 5, - targetGrade: 2, - submittedPercentage: 0, - completedPercentage: 0, - distributionAvailable: true, - statusDistribution: [ - {status: 'not_started', percentage: 50}, - {status: 'feedback_exceeded', percentage: 0}, - {status: 'redo', percentage: 0}, - {status: 'need_help', percentage: 10}, - {status: 'working_on_it', percentage: 30}, - {status: 'fix_and_resubmit', percentage: 10}, - {status: 'ready_for_feedback', percentage: 0}, - {status: 'discuss', percentage: 0}, - {status: 'demonstrate', percentage: 0}, - {status: 'complete', percentage: 0}, - {status: 'fail', percentage: 0}, - {status: 'time_exceeded', percentage: 0}, - {status: 'assess_in_portfolio', percentage: 0}, - {status: 'attention_required', percentage: 0}, - {status: 'rediscuss', percentage: 0}, - ], - isUserEnabled: true, - isSuppressed: false, - isStale: false, - isFeatureEnabled: true, - lastUpdatedAt: '2026-08-14T03:15:00Z', - unavailableMessage: '', - unavailableReason: null, - distributionUnavailableReason: null, -}; - -export const FIXTURE_SUPPRESSED: PeerProgressIndicator = { - taskDefinitionId: 12, - unitId: 5, - targetGrade: 2, - submittedPercentage: null, - completedPercentage: null, - distributionAvailable: false, - statusDistribution: [], - isUserEnabled: true, - isSuppressed: true, - isStale: false, - isFeatureEnabled: true, - lastUpdatedAt: '2026-08-14T03:15:00Z', - unavailableMessage: 'Not enough students to show progress.', - unavailableReason: 'insufficient_cohort', - distributionUnavailableReason: 'insufficient_cohort', -}; - -export const FIXTURE_UNAVAILABLE: PeerProgressIndicator = { - taskDefinitionId: 12, - unitId: 5, - targetGrade: 2, - submittedPercentage: null, - completedPercentage: null, - distributionAvailable: false, - statusDistribution: [], - isUserEnabled: true, - isSuppressed: false, - isStale: false, - isFeatureEnabled: true, - lastUpdatedAt: '2026-08-14T03:15:00Z', - unavailableMessage: 'Progress unavailable.', - unavailableReason: 'snapshot_unavailable', - distributionUnavailableReason: 'snapshot_unavailable', -}; - -export const FIXTURE_DISABLED: PeerProgressIndicator = { - taskDefinitionId: 12, - unitId: 5, - targetGrade: 2, - submittedPercentage: null, - completedPercentage: null, - distributionAvailable: false, - statusDistribution: [], - isUserEnabled: true, - isSuppressed: false, - isStale: false, - isFeatureEnabled: false, - lastUpdatedAt: '2026-08-14T03:15:00Z', - unavailableMessage: 'Peer Progress Indicator is disabled for this unit.', - unavailableReason: 'feature_disabled', - distributionUnavailableReason: 'feature_disabled', -}; - -export const FIXTURE_STALE: PeerProgressIndicator = { - taskDefinitionId: 12, - unitId: 5, - targetGrade: 2, - submittedPercentage: null, - completedPercentage: null, - distributionAvailable: false, - statusDistribution: [], - isUserEnabled: true, - isSuppressed: false, - isStale: true, - isFeatureEnabled: true, - lastUpdatedAt: '2026-08-14T03:15:00Z', - unavailableMessage: 'Progress data is stale.', - unavailableReason: 'stale', - distributionUnavailableReason: 'stale', -}; - -// Malformed fixture for safe-failure tests -export const FIXTURE_MALFORMED: unknown = { - taskDefinitionId: null, - unitId: undefined, - targetGrade: 'wrong-type', - submittedPercentage: NaN, - completedPercentage: -1, - distributionAvailable: 'yes', - statusDistribution: [{status: 'not_real', percentage: 200}], - isUserEnabled: 'yes', - isSuppressed: 'nope', - isStale: 123, - isFeatureEnabled: 'false', - lastUpdatedAt: 'not-a-date', - unavailableMessage: 999, - unavailableReason: 'raw server detail', - distributionUnavailableReason: 'raw server detail', -}; diff --git a/src/app/api/models/notification.ts b/src/app/api/models/notification.ts index 33ac7f2444..0fe9ede99d 100644 --- a/src/app/api/models/notification.ts +++ b/src/app/api/models/notification.ts @@ -4,9 +4,10 @@ import {Entity} from 'ngx-entity-service'; * Something that happened which the signed in user should be told about. * * `notificationType` is the category the user's preferences switch on, one of - * task, feedback, portfolio, extension or general. The api gates delivery on - * the first three against the receive_*_notifications columns, so the web app - * never has to check a preference before rendering one of these. + * task, feedback, portfolio, extension, general or unit_hub. The api gates + * delivery on task, feedback, portfolio and unit_hub against the + * receive_*_notifications columns, so the web app never has to check a + * preference before rendering one of these. * * `event` is the stable, specific hook (for example `task_comment_created`). * Presentation can vary by event without inferring meaning from message text. @@ -42,6 +43,13 @@ export class Notification extends Entity { commentId?: number | null; groupId?: number | null; + /** + * The Unit Hub announcement or session a unit_hub notification is about. + * Null once it has been deleted, undefined from an api that predates them. + */ + announcementId?: number | null; + sessionId?: number | null; + /** * When the user read this, or null while it is still unread. */ diff --git a/src/app/api/models/task-comment/task-comment.ts b/src/app/api/models/task-comment/task-comment.ts index 1508518b34..be3d9d5fb8 100644 --- a/src/app/api/models/task-comment/task-comment.ts +++ b/src/app/api/models/task-comment/task-comment.ts @@ -22,6 +22,12 @@ export class TaskComment extends Entity { recipientReadTime: string; commentType: string = 'text'; isNew: boolean; + /** + * True when OnTrack wrote this comment rather than a person. It is stored + * against the tutor for the task, because a comment needs an author, so + * without this it reads as something that tutor said. + */ + automated = false; replyToId: number; attachmentFileName?: string; attachmentMimeType?: string; diff --git a/src/app/api/models/task.ts b/src/app/api/models/task.ts index 5187c827a6..843cd511a7 100644 --- a/src/app/api/models/task.ts +++ b/src/app/api/models/task.ts @@ -4,6 +4,8 @@ import {HttpClient} from '@angular/common/http'; import {LOCALE_ID} from '@angular/core'; import {Observable, finalize, firstValueFrom, map} from 'rxjs'; import {AppInjector} from 'src/app/app-injector'; +import {SubmissionCelebrationService} from 'src/app/common/celebrate/submission-celebration.service'; +import type {SubmissionCelebration} from 'src/app/common/celebrate/submission-timing'; import {AlertService} from 'src/app/common/services/alert.service'; import {DoubtfireConstants} from 'src/app/config/constants/doubtfire-constants'; import {GradeTaskModalService} from 'src/app/tasks/modals/grade-task-modal/grade-task-modal.service'; @@ -124,6 +126,9 @@ export class Task extends Entity { suggestedTaskStatus; + /** Status held before the student's current submission began. Cleared once it settles. */ + private statusBeforeSubmission?: TaskStatusEnum; + private _unit: Unit; constructor(data?: Project | Unit) { @@ -978,6 +983,9 @@ export class Task extends Entity { isTestSubmission: boolean = false, ) { const oldStatus = this.status; + // A new submission remembers where it came from, so a resubmission reads as one. + // New evidence and test submissions are not status changes and are not celebrated. + this.statusBeforeSubmission = !reuploadEvidence && !isTestSubmission ? oldStatus : undefined; if (!isTestSubmission) { this.status = status; @@ -987,6 +995,7 @@ export class Task extends Entity { const modal = uploadModal.show(this, reuploadEvidence, isTestSubmission); // Modal failed to present if (!modal) { + this.statusBeforeSubmission = undefined; if (!isTestSubmission) { this.status = oldStatus; } @@ -1000,6 +1009,7 @@ export class Task extends Entity { }, // Grade was not selected (modal was dismissed) (_dismissed) => { + this.statusBeforeSubmission = undefined; if (!isTestSubmission) { this.status = oldStatus; } @@ -1009,11 +1019,18 @@ export class Task extends Entity { ); } + /** + * Set `claimCelebration` when the caller has a surface of its own to show the + * confirmation in, such as the submission dialog it was started from. The + * celebration is returned instead of being raised over the page, and the + * snackbar stays suppressed either way. + */ public processTaskStatusChange( expectedStatus: TaskStatusEnum, alerts: AlertService, submissionCompleted: boolean = false, - ) { + claimCelebration: boolean = false, + ): SubmissionCelebration | null { if (this.inTimeExceeded() && !this.isPastDeadline()) { alerts.message( 'You have submitted after the deadline for feedback. Your task will not be reviewed by a tutor. It is now your responsibility to ensure this task meets the required standard.', @@ -1021,14 +1038,52 @@ export class Task extends Entity { ); } + const previousStatus = this.statusBeforeSubmission; + this.statusBeforeSubmission = undefined; + const eligible = + submissionCompleted && + expectedStatus === 'ready_for_feedback' && + previousStatus !== undefined; + + let claimed: SubmissionCelebration | null = null; + let celebrated = false; + if (eligible) { + if (claimCelebration) { + claimed = this.describeSubmissionCelebration(previousStatus); + celebrated = claimed !== null; + } else { + celebrated = this.celebrateSubmission(previousStatus); + } + } + if (this.status !== expectedStatus) { alerts.message(`Status changed to ${this.statusLabel()}.`, 4000); - } else { + } else if (!celebrated) { + // The submission confirmation already says this, so it replaces the snackbar. alerts.success(`Status changed to ${this.statusLabel()}.`); } this.getSubmissionDetails().subscribe(); const taskService: TaskService = AppInjector.get(TaskService); taskService.notifyTransitionComplete(this, submissionCompleted); + return claimed; + } + + private celebrateSubmission(previousStatus: TaskStatusEnum): boolean { + try { + return AppInjector.get(SubmissionCelebrationService).celebrate(this, previousStatus); + } catch { + return false; + } + } + + private describeSubmissionCelebration( + previousStatus: TaskStatusEnum, + ): SubmissionCelebration | null { + try { + return AppInjector.get(SubmissionCelebrationService).describe(this, previousStatus); + } catch { + return null; + } } public async markAsDiscussed(reasonText?: string) { @@ -1152,6 +1207,7 @@ export class Task extends Entity { this.project.taskCache.delete(this.definition.abbreviation); this.project.taskCache.add(this); } + this.statusBeforeSubmission = submissionCompleted ? oldStatus : undefined; this.processTaskStatusChange(status, alerts, submissionCompleted); taskService.notifyStatusChange(this); }, diff --git a/src/app/api/models/unit.reviewed.spec.ts b/src/app/api/models/unit.reviewed.spec.ts index 8222062ca4..1a1023fdaa 100644 --- a/src/app/api/models/unit.reviewed.spec.ts +++ b/src/app/api/models/unit.reviewed.spec.ts @@ -33,27 +33,3 @@ describe('Unit.findStudent', () => { expect(unit.findStudent(999)).toBeUndefined(); }); }); - -describe('Unit.findStudent (via stubbed cache)', () => { - // Build a Unit without running the constructor so the test stays free of the - // Angular injector, then give it a stub studentCache. - function unitWithCache(entries: Record): Unit { - const unit = Object.create(Unit.prototype) as Unit; - // eslint-disable-next-line @typescript-eslint/no-explicit-any - (unit as any).studentCache = { - get: (key: number) => entries[key], - }; - return unit; - } - - it('returns the same instance the cache holds for that id', () => { - const project = {id: 7} as unknown; - const unit = unitWithCache({7: project}); - expect(unit.findStudent(7)).toBe(project); - }); - - it('returns undefined when no project has that id', () => { - const unit = unitWithCache({7: {id: 7}}); - expect(unit.findStudent(99)).toBeUndefined(); - }); -}); diff --git a/src/app/api/models/user/user.ts b/src/app/api/models/user/user.ts index 0c9d816100..ff95beb75c 100644 --- a/src/app/api/models/user/user.ts +++ b/src/app/api/models/user/user.ts @@ -19,6 +19,13 @@ export class User extends Entity { public receiveTaskNotifications: boolean; public receivePortfolioNotifications: boolean; public receiveFeedbackNotifications: boolean; + /** Unit Hub updates in the bell. Email, push and reminders only apply while this is on. */ + public receiveUnitHubNotifications: boolean; + public receiveUnitHubEmailNotifications: boolean; + public receiveUnitHubPushNotifications: boolean; + public receiveUnitHubSessionReminders: boolean; + /** How often the unit summary email arrives: off, daily, weekly or monthly. */ + public digestFrequency: string; public displayPeerProgress: boolean; public themePreference: 'light' | 'dark' | 'system' | null; public themePreferenceUpdatedAt: string | null; diff --git a/src/app/api/services/notification-route.service.ts b/src/app/api/services/notification-route.service.ts index 6fe75061e7..8af00ad2aa 100644 --- a/src/app/api/services/notification-route.service.ts +++ b/src/app/api/services/notification-route.service.ts @@ -17,6 +17,8 @@ const FORBIDDEN_ROUTE_TEXT = /[\s\\?#%]/; const PROJECT_ROOT_ROUTE = /^\/projects\/[1-9]\d*\/(?:dashboard|groups)$/; const PROJECT_TASK_ROUTE = /^\/projects\/[1-9]\d*\/dashboard\/[A-Za-z0-9][A-Za-z0-9._-]{0,31}(?:\/feedback)?$/; +// The one destination allowed a query string, and only as two numeric ids. +const UNIT_HUB_ROUTE = /^\/unit-hub\?unit=[1-9]\d{0,9}&(?:announcement|session)=[1-9]\d{0,9}$/; const PROJECT_FEEDBACK_ROUTE = /^\/projects\/([1-9]\d*)\/dashboard\/([A-Za-z0-9][A-Za-z0-9._-]{0,31})\/feedback$/; @@ -49,6 +51,9 @@ export class NotificationRouteService { if (!link.startsWith('/') || link.startsWith('//')) { return NOTIFICATION_ROUTE_FALLBACK; } + if (UNIT_HUB_ROUTE.test(link)) { + return link; + } if (hasControlCharacters(link) || FORBIDDEN_ROUTE_TEXT.test(link)) { return NOTIFICATION_ROUTE_FALLBACK; } diff --git a/src/app/api/services/spec/notification-route.service.spec.ts b/src/app/api/services/spec/notification-route.service.spec.ts index 502bf39ee2..ae17a4f2e8 100644 --- a/src/app/api/services/spec/notification-route.service.spec.ts +++ b/src/app/api/services/spec/notification-route.service.spec.ts @@ -41,6 +41,8 @@ describe('NotificationRouteService', () => { '/projects/23/dashboard/P-2.21', '/projects/23/dashboard/D-9.568', '/projects/23/dashboard/C-4.602', + '/unit-hub?unit=3&announcement=12', + '/unit-hub?unit=3&session=40', ]; for (const route of approved) { @@ -118,6 +120,14 @@ describe('NotificationRouteService', () => { '/projects/1/dashboard/1.1P?token=secret', '/projects/1/dashboard/1.1P#feedback', '/notifications?student=Alice', + '/unit-hub', + '/unit-hub?unit=3', + '/unit-hub?announcement=12&unit=3', + '/unit-hub?unit=0&announcement=12', + '/unit-hub?unit=3&announcement=12&next=//evil.test', + '/unit-hub?unit=3&session=40#details', + '/unit-hub?unit=3&task=12', + '/unit-hub?unit=3&session=%34%30', ]; for (const value of invalid) { diff --git a/src/app/api/services/task-comment.service.ts b/src/app/api/services/task-comment.service.ts index c694ab41fb..cbe2fa1f75 100644 --- a/src/app/api/services/task-comment.service.ts +++ b/src/app/api/services/task-comment.service.ts @@ -75,6 +75,7 @@ export class TaskCommentService extends CachedEntityService { 'recipientReadTime', 'replyToId', 'isNew', + 'automated', 'attachmentFileName', 'attachmentMimeType', 'attachmentByteSize', diff --git a/src/app/api/services/user.service.ts b/src/app/api/services/user.service.ts index dd096cba3f..a0e96d1acc 100644 --- a/src/app/api/services/user.service.ts +++ b/src/app/api/services/user.service.ts @@ -32,6 +32,11 @@ export class UserService extends CachedEntityService { 'receiveTaskNotifications', 'receivePortfolioNotifications', 'receiveFeedbackNotifications', + 'receiveUnitHubNotifications', + 'receiveUnitHubEmailNotifications', + 'receiveUnitHubPushNotifications', + 'receiveUnitHubSessionReminders', + 'digestFrequency', 'displayPeerProgress', 'themePreference', 'themePreferenceUpdatedAt', diff --git a/src/app/app.routes.ts b/src/app/app.routes.ts index cfab2142f1..0b10cdebf0 100644 --- a/src/app/app.routes.ts +++ b/src/app/app.routes.ts @@ -84,6 +84,24 @@ export const routes: Routes = [ import('./common/theme/theme-demo.component').then((m) => m.ThemeDemoComponent), data: {pageTitle: 'Theme foundation'}, }, + { + path: 'motion-demo', + loadComponent: () => + import('./common/celebrate/motion-demo/motion-demo.component').then( + (m) => m.MotionDemoComponent, + ), + canActivate: [demoToolsGuard], + data: {pageTitle: 'Motion'}, + }, + { + path: 'submit-motion', + loadComponent: () => + import('./common/celebrate/motion-demo/submit-motion.component').then( + (m) => m.SubmitMotionComponent, + ), + canActivate: [demoToolsGuard], + data: {pageTitle: 'Submitting'}, + }, { path: 'demo-controls', component: DemoControlsComponent, diff --git a/src/app/common/additional-notification-email/additional-notification-email.component.html b/src/app/common/additional-notification-email/additional-notification-email.component.html index eaf82e7f51..08e856e42a 100644 --- a/src/app/common/additional-notification-email/additional-notification-email.component.html +++ b/src/app/common/additional-notification-email/additional-notification-email.component.html @@ -1,7 +1,9 @@ + + + + @if (showActions(form.dirty)) { +
@if (form.invalid && form.dirty) {

Fix the highlighted fields before saving.

+ } @else if (mode === 'edit' && form.dirty) { +

You have unsaved changes.

} -

{{ saveMessage }}

+

+ {{ saveMessage }} +

{{ saveError }}

+ + + @if (mode === 'create') { + + } + @if (mode === 'edit' && (form.dirty || saving)) { + + + }
- + } diff --git a/src/app/common/edit-profile-form/edit-profile-form.component.scss b/src/app/common/edit-profile-form/edit-profile-form.component.scss index 01a748bf22..432429c20b 100644 --- a/src/app/common/edit-profile-form/edit-profile-form.component.scss +++ b/src/app/common/edit-profile-form/edit-profile-form.component.scss @@ -1,76 +1,120 @@ +// Profile page, first-login form and the admin Users dialog. Cards stack with one +// 16px rhythm and no rules between sections. Colours come only from --ot-* tokens. +@use './profile-form' as profile; + +@include profile.theme; + :host { display: block; height: auto; + min-width: 0; + // the save bar's height, so a control focused from the keyboard scrolls clear of it + --profile-actions-height: 68px; } -form, -.main-container, -.main { +.profile-form { + display: flex; + flex-direction: column; + box-sizing: border-box; + width: 100%; max-width: 100%; + min-width: 0; + padding-inline: clamp(16px, 4vw, 28px); + color: var(--ot-color-text); } -.main { - gap: 4px; - padding: 16px clamp(16px, 4vw, 28px) calc(3rem + env(safe-area-inset-bottom)); +.profile-form--modal { + padding: 24px 24px 0; + + @media (max-width: 599px) { + padding: 16px 16px 0; + } } +.profile-stack { + display: flex; + flex-direction: column; + gap: 16px; + min-width: 0; + padding-block: 16px; -h1 { - // THM-M01: was hardcoded black. - color: var(--ot-color-text, #000); - margin-top: 0; + :is(input, textarea, button, a, mat-select, mat-checkbox, mat-slide-toggle) { + scroll-margin-block: 16px calc(var(--profile-actions-height) + 16px); + } } -a { - :hover { - text-decoration: none; - opacity: 0.6; - } +.profile-card { + @include profile.card; } -p { - color: var(--ot-color-text, #000); +.profile-card__heading { + @include profile.card-heading; } -.form-field { - margin: 5px; +.profile-fields { + @include profile.field-grid; } -section { - padding: 12px; +.profile-fields__wide { + grid-column: 1 / -1; } -.account-information { - background: var(--ot-color-surface); - color: var(--ot-color-text); - border: 1px solid var(--ot-color-border); - border-radius: 12px; - display: grid; - gap: 12px; - margin-bottom: 16px; - padding: 16px; - width: 100%; +// ---- Header ---- +.profile-header { + flex-direction: row; + align-items: center; + gap: 16px; +} - h2, - p, - dl, - dd { - margin: 0; +.profile-header__avatar { + display: block; + flex-shrink: 0; + width: 64px; + height: 64px; + border-radius: var(--ot-radius-circle); + transition: opacity 160ms ease; + + &:hover { + opacity: 0.8; } +} + +.profile-header__text { + display: flex; + flex-direction: column; + gap: 2px; + min-width: 0; - h2 { - font-size: 1.125rem; + h1 { + margin: 0; + color: var(--ot-color-text); + font-size: 1.5rem; + font-weight: 650; + line-height: 1.3; + letter-spacing: -0.01em; } p { + margin: 0; color: var(--ot-color-text-muted); - margin-top: 4px; + font-size: 0.9rem; + line-height: 1.45; } +} +.profile-header__meta { + display: flex; + flex-wrap: wrap; + gap: 0 12px; +} + +// ---- Account facts ---- +.account-information { dl { display: grid; - gap: 10px; - grid-template-columns: repeat(3, minmax(0, 1fr)); + gap: 16px 24px; + grid-template-columns: repeat(auto-fit, minmax(min(100%, 12rem), 1fr)); + margin: 0; } dl > div { @@ -78,97 +122,133 @@ section { } dt { + margin: 0 0 4px; color: var(--ot-color-text-muted); - font-size: 0.8125rem; + font-size: 0.72rem; font-weight: 600; + letter-spacing: 0.04em; + line-height: 1.4; + text-transform: uppercase; } dd { + margin: 0; + color: var(--ot-color-text); + font-size: 0.95rem; overflow-wrap: anywhere; } - - &__managed { - border-top: 1px solid var(--ot-color-divider); - padding-top: 10px; - } } -/* checkbox */ -::ng-deep .mdc-form-field label { - font-weight: normal; +.account-information__managed { + margin: 0; + color: var(--ot-color-text-muted); + font-size: 0.875rem; } -.mat-icon { - width: 50px; - height: 50px; - font-size: 50px; +// ---- Setting rows ---- +.profile-setting { + @include profile.setting-row; } -.push-opt-in { - // The rule above sizes the profile avatar icon at 50px. Left alone it would - // apply to the icon inside this button too and blow the button apart. - .mat-icon { - width: 20px; - height: 20px; - font-size: 20px; - margin-right: 6px; - } +.push-opt-in__body { + display: flex; + flex-direction: column; + align-items: flex-start; + gap: 8px; + min-width: 0; - &__hint { - display: block; - margin-top: 6px; - opacity: 0.7; + button { + max-width: 100%; + height: auto; + min-height: 44px; + white-space: normal; } } -.peer-progress-preference { - // Divider token equals the raised surface in dark (#353c47), so this separator - // vanishes on the dialog. Use the border token in dark where it actually reads; - // keep the softer divider in light. - border-block: 1px solid var(--ot-color-divider); - :root[data-ot-theme='dark'] & { - border-block-color: var(--ot-color-border); - } - gap: 6px; - margin-block: 8px; - padding-block: 16px; - - h3 { - margin: 0; - } +.push-opt-in__hint { + display: block; + color: var(--ot-color-text-muted); + font-size: 0.84rem; + line-height: 1.45; +} - small { - color: var(--ot-color-text-muted); - max-width: 42rem; - } +.push-opt-in__instructions { + margin: 0; + padding-inline-start: 1.25rem; + color: var(--ot-color-text-muted); + font-size: 0.84rem; + line-height: 1.5; } +// ---- Save bar ---- +// Only on screen when there is something to act on, so it arrives as an answer to +// an edit rather than sitting there as chrome. Sticky at the foot of the content +// column, in flow after the last card, so at the end of the page it covers nothing. .profile-actions { - background: var(--ot-color-surface-raised); - border-top: 1px solid var(--ot-color-divider); - bottom: 0; - padding: 12px clamp(16px, 4vw, 28px) max(12px, env(safe-area-inset-bottom)); position: sticky; - width: 100%; + bottom: 0; z-index: 2; + display: flex; + flex-wrap: wrap; + align-items: center; + justify-content: flex-end; + gap: 8px 12px; + box-sizing: border-box; + min-height: var(--profile-actions-height); + margin-block-start: 4px; + padding: 12px 16px max(12px, env(safe-area-inset-bottom, 0px)); + border: 1px solid var(--ot-color-border); + border-radius: var(--ot-radius-md); + // Raised, not a full-bleed band: it lines up with the cards above it rather + // than reading as a separate strip pinned to the window. + background: var(--ot-color-surface-raised); + box-shadow: var(--ot-elevation-1, 0 1px 2px rgb(0 0 0 / 12%)); + animation: profile-actions-in 200ms cubic-bezier(0.23, 1, 0.32, 1) both; - button { - min-height: 44px; + @media (max-width: 599px) { + padding-inline: 16px; + + > button { + flex: 1 1 100%; + } + } +} + +.profile-actions__saved { + display: flex; + align-items: center; + gap: 6px; + min-height: 44px; + margin: 0; + color: var(--ot-color-text-muted); + font-size: 0.875rem; + + mat-icon { + width: 18px; + height: 18px; + font-size: 18px; } } .profile-save-status { - min-height: 24px; - width: 100%; + flex: 1 1 12rem; + min-width: 0; + font-size: 0.875rem; + line-height: 1.45; p { - margin: 4px 0 0; + margin: 0; } &__success { color: var(--ot-color-success); } + &__pending { + color: var(--ot-color-text); + font-weight: 600; + } + &__validation, &__error { color: var(--ot-color-error); @@ -176,33 +256,30 @@ section { } } -.profile-name-fields { - display: grid; - gap: 16px; - grid-template-columns: repeat(2, minmax(0, 1fr)); - width: 100%; +// Nothing left to do, just the confirmation on its way out. +.profile-actions--settled { + justify-content: flex-start; - mat-form-field { - min-width: 0; - width: 100%; + .profile-save-status__success { + color: var(--ot-color-success); + font-weight: 600; } } -@media (max-width: 600px) { - .profile-name-fields { - gap: 0; - grid-template-columns: 1fr; +@keyframes profile-actions-in { + from { + opacity: 0; + transform: translateY(8px); } - .main { - padding-inline: 16px; - } - - .account-information dl { - grid-template-columns: 1fr; + to { + opacity: 1; + transform: none; } +} +@media (prefers-reduced-motion: reduce) { .profile-actions { - padding-inline: 16px; + animation: none; } } diff --git a/src/app/common/edit-profile-form/edit-profile-form.component.spec.ts b/src/app/common/edit-profile-form/edit-profile-form.component.spec.ts index cfd2dce546..899ac6d339 100644 --- a/src/app/common/edit-profile-form/edit-profile-form.component.spec.ts +++ b/src/app/common/edit-profile-form/edit-profile-form.component.spec.ts @@ -4,7 +4,7 @@ import {ComponentFixture, TestBed} from '@angular/core/testing'; import {MAT_DIALOG_DATA} from '@angular/material/dialog'; import {MatSnackBar} from '@angular/material/snack-bar'; import {Router} from '@angular/router'; -import {of} from 'rxjs'; +import {Subject, of} from 'rxjs'; import {User} from 'src/app/api/models/user/user'; import {AuthenticationService} from 'src/app/api/services/authentication.service'; import {PushNotificationService} from 'src/app/api/services/push-notification.service'; @@ -228,10 +228,56 @@ describe('EditProfileFormComponent', () => { expect(component.canEditStudentId).toBe(true); }); + it('treats an SSO name as read-only and a local one as editable', () => { + dialogData.user = makeUser({institutionalIdentityManaged: true, emailEditable: false}); + createComponent(); + expect(component.canEditName).toBe(false); + + dialogData.user = makeUser({institutionalIdentityManaged: false, emailEditable: true}); + createComponent(); + expect(component.canEditName).toBe(true); + }); + + it('previews the institution-managed page from a dev-only query parameter', () => { + const search = window.location.search; + window.history.replaceState({}, '', `${window.location.pathname}?identityManaged=1`); + try { + dialogData.user = makeUser({institutionalIdentityManaged: false, emailEditable: true}); + createComponent(); + expect(component.canEditName).toBe(false); + expect(component.canEditEmail).toBe(false); + expect(component.identityManagedView).toBe(true); + } finally { + window.history.replaceState({}, '', `${window.location.pathname}${search}`); + } + }); + + it('leaves institution-managed name and email out of the update', () => { + const user = makeUser({ + institutionalIdentityManaged: true, + emailEditable: false, + nickname: 'Preferred', + }); + dialogData.user = user; + userServiceStub.update.mockReturnValue(of(user)); + + createComponent(); + component.submit(); + + expect(userServiceStub.update).toHaveBeenCalledWith(user, { + entity: user, + ignoreKeys: ['firstName', 'lastName', 'email'], + }); + }); + it('reports explicit saving and success state while preserving genuine settings', () => { + // Database auth, so nothing on this account is institution-managed and the + // whole entity goes up. The SSO case is covered separately below. const updated = makeUser({ nickname: 'Preferred', receiveFeedbackNotifications: false, + institutionalIdentityManaged: false, + emailEditable: true, }); dialogData.user = updated; userServiceStub.update.mockReturnValue(of(updated)); @@ -245,6 +291,39 @@ describe('EditProfileFormComponent', () => { expect(component.user.nickname).toBe('Preferred'); expect(component.user.receiveFeedbackNotifications).toBe(false); }); + + it('lets a save land after the view goes, but arms no timer behind it', () => { + vi.useFakeTimers(); + + // A save that has left but has not been answered yet. + const response: Subject = new Subject(); + userServiceStub.update.mockReturnValue(response); + + createComponent(); + component.submit(); + expect(component.saving).toBe(true); + + // The user leaves the profile page before the server replies. ngOnDestroy + // has already had its one chance to clear the confirmation timer, so a + // next handler running after this point would arm one nothing can clear. + // The request is deliberately not cancelled: the save itself must survive. + fixture.destroy(); + const pendingAfterDestroy = vi.getTimerCount(); + expect(response.observed).toBe(true); + + response.next(makeUser({nickname: 'Preferred'})); + response.complete(); + + expect(component.justSaved).toBe(false); + expect(component.saveMessage).toBe(''); + expect(vi.getTimerCount()).toBe(pendingAfterDestroy); + + // And nothing turns up later either. + vi.advanceTimersByTime(10_000); + expect(component.justSaved).toBe(false); + + vi.useRealTimers(); + }); }); // A11Y-FORM06: WCAG 1.3.5 Identify Input Purpose (AA). @@ -407,3 +486,217 @@ describe('EditProfileFormComponent notifications page link', () => { expect(notificationsLink()).toBeNull(); }); }); + +// The save bar only exists while there is something to act on. +@Directive({selector: 'form', exportAs: 'ngForm', standalone: false}) +class StubNgFormSaveBar { + public invalid = false; + public dirty = false; + public pristine = true; +} + +describe('EditProfileFormComponent save bar and labels', () => { + let fixture: ComponentFixture; + + beforeEach(async () => { + const currentUser = makeUser({firstName: 'Ada', lastName: 'Lovelace', username: 'ada'}); + + await TestBed.configureTestingModule({ + declarations: [EditProfileFormComponent, StubNgFormSaveBar, StubNotificationNgModel], + providers: [ + {provide: AlertService, useValue: {error: vi.fn()}}, + { + provide: DoubtfireConstants, + useValue: {ExternalName: {value: 'OnTrack'}, IsTiiEnabled: {value: false}}, + }, + {provide: UserService, useValue: {currentUser}}, + {provide: Router, useValue: {}}, + {provide: AuthenticationService, useValue: {}}, + {provide: MAT_DIALOG_DATA, useValue: null}, + {provide: MatSnackBar, useValue: {}}, + {provide: PushNotificationService, useValue: pushServiceStub}, + ], + schemas: [NO_ERRORS_SCHEMA], + }).compileComponents(); + + fixture = TestBed.createComponent(EditProfileFormComponent); + fixture.componentRef.setInput('mode', 'edit'); + fixture.detectChanges(); + }); + + const form = (): StubNgFormSaveBar => + fixture.debugElement.children[0].injector.get(StubNgFormSaveBar); + const text = (): string => fixture.nativeElement.textContent; + + it('shows no bar at all when there is nothing to act on', () => { + expect(fixture.nativeElement.querySelector('.profile-actions')).toBeNull(); + expect(fixture.nativeElement.querySelector('button[type="submit"]')).toBeNull(); + expect(text()).not.toContain('All changes saved'); + }); + + it('brings the bar in with both actions once the form has changes', () => { + form().dirty = true; + form().pristine = false; + fixture.detectChanges(); + + expect(fixture.nativeElement.querySelector('.profile-actions')).not.toBeNull(); + expect(text()).toContain('You have unsaved changes'); + + const submit: HTMLButtonElement = fixture.nativeElement.querySelector('button[type="submit"]'); + expect(submit.textContent).toContain('Save changes'); + expect(text()).toContain('Discard'); + }); + + it('keeps the bar while a save is in flight and while the result is still showing', () => { + const component = fixture.componentInstance; + + component.saving = true; + fixture.detectChanges(); + expect(fixture.nativeElement.querySelector('.profile-actions')).not.toBeNull(); + + component.saving = false; + component.justSaved = true; + component.saveMessage = 'Profile saved.'; + fixture.detectChanges(); + expect(text()).toContain('Profile saved.'); + + // Once the confirmation has had its moment the bar goes with it. + component.justSaved = false; + component.saveMessage = ''; + fixture.detectChanges(); + expect(fixture.nativeElement.querySelector('.profile-actions')).toBeNull(); + }); + + it('puts every edited field back when the changes are discarded', () => { + const component = fixture.componentInstance; + const original = component.user.firstName; + + component.user.firstName = 'Edited'; + component.user.nickname = 'Edited too'; + component.discard(); + + expect(component.user.firstName).toBe(original); + expect(component.user.nickname).not.toBe('Edited too'); + }); + + it('labels the name fields in sentence case', () => { + const labels = Array.from( + fixture.nativeElement.querySelectorAll( + '.profile-name-fields mat-label', + ) as NodeListOf, + ).map((label) => label.textContent.trim()); + + expect(labels).toEqual(['First name', 'Last name', 'Preferred name', 'Custom pronouns']); + expect(text()).not.toContain('Second Name'); + }); + + it('puts the display name and username in the header', () => { + expect(fixture.nativeElement.querySelector('.profile-header h1').textContent.trim()).toBe( + 'Ada Lovelace', + ); + expect(fixture.nativeElement.querySelector('.profile-header__meta').textContent).toContain( + 'ada', + ); + }); +}); + +// Identity the institution asserts is shown, not offered for editing. Renders +// the real template under both auth methods, because the difference between the +// two is entirely in what the form puts on the page. +describe('EditProfileFormComponent institution-managed identity', () => { + let fixture: ComponentFixture; + + const render = async (user: User): Promise => { + TestBed.resetTestingModule(); + + await TestBed.configureTestingModule({ + declarations: [EditProfileFormComponent, StubNgFormProfile, StubNotificationNgModel], + providers: [ + {provide: AlertService, useValue: {error: vi.fn()}}, + { + provide: DoubtfireConstants, + useValue: {ExternalName: {value: 'OnTrack'}, IsTiiEnabled: {value: false}}, + }, + {provide: UserService, useValue: {currentUser: user}}, + {provide: Router, useValue: {}}, + {provide: AuthenticationService, useValue: {}}, + {provide: MAT_DIALOG_DATA, useValue: {user, mode: 'edit', modal: false}}, + {provide: MatSnackBar, useValue: {}}, + {provide: PushNotificationService, useValue: pushServiceStub}, + ], + schemas: [NO_ERRORS_SCHEMA], + }).compileComponents(); + + fixture = TestBed.createComponent(EditProfileFormComponent); + fixture.detectChanges(); + }; + + const input = (name: string): Element | null => + fixture.nativeElement.querySelector(`input[name="${name}"]`); + const accountFacts = (): string[] => + Array.from( + fixture.nativeElement.querySelectorAll('.account-information dt') as NodeListOf, + ).map((term) => term.textContent.trim()); + + const ssoUser = (): User => + makeUser({ + firstName: 'Ada', + lastName: 'Lovelace', + username: 'ada', + email: 'ada@institution.edu', + nickname: 'Addy', + institutionalIdentityManaged: true, + emailEditable: false, + }); + + const localUser = (): User => + makeUser({ + firstName: 'Ada', + lastName: 'Lovelace', + username: 'ada', + email: 'ada@local.test', + institutionalIdentityManaged: false, + emailEditable: true, + }); + + it('shows the managed name and email as account facts under SSO', async () => { + await render(ssoUser()); + + expect(accountFacts()).toContain('First name'); + expect(accountFacts()).toContain('Last name'); + expect(accountFacts()).toContain('Institutional / sign-in email'); + expect(fixture.nativeElement.querySelector('.account-information dl').textContent).toContain( + 'Lovelace', + ); + expect(input('first')).toBeNull(); + expect(input('last')).toBeNull(); + expect(input('email')).toBeNull(); + }); + + it('explains where the managed details come from under SSO', async () => { + await render(ssoUser()); + + expect( + fixture.nativeElement.querySelector('.account-information__managed').textContent, + ).toContain('Your name and email come from your institution account.'); + }); + + it('keeps the preferred name editable and hinted under SSO', async () => { + await render(ssoUser()); + + expect(input('preferred_name')).not.toBeNull(); + expect(fixture.nativeElement.textContent).toContain( + 'Shown to your tutors instead of your first name.', + ); + }); + + it('keeps name and email editable under database auth', async () => { + await render(localUser()); + + expect(input('first')).not.toBeNull(); + expect(input('last')).not.toBeNull(); + expect(input('email')).not.toBeNull(); + expect(accountFacts()).not.toContain('First name'); + expect(fixture.nativeElement.querySelector('.account-information__managed')).toBeNull(); + }); +}); diff --git a/src/app/common/edit-profile-form/edit-profile-form.component.ts b/src/app/common/edit-profile-form/edit-profile-form.component.ts index 7760497140..94002347b6 100644 --- a/src/app/common/edit-profile-form/edit-profile-form.component.ts +++ b/src/app/common/edit-profile-form/edit-profile-form.component.ts @@ -7,6 +7,7 @@ import { OnDestroy, OnInit, Optional, + isDevMode, } from '@angular/core'; import {NgForm} from '@angular/forms'; import {MAT_DIALOG_DATA} from '@angular/material/dialog'; @@ -20,6 +21,9 @@ import {UserService} from 'src/app/api/services/user.service'; import {AlertService} from 'src/app/common/services/alert.service'; import {DoubtfireConstants} from 'src/app/config/constants/doubtfire-constants'; +/** How long the save confirmation stays before the bar leaves with it. */ +const SAVED_CONFIRMATION_MS = 2600; + @Component({ selector: 'f-edit-profile-form', templateUrl: './edit-profile-form.component.html', @@ -58,6 +62,8 @@ export class EditProfileFormComponent implements OnInit, OnDestroy { public formPronouns = {pronouns: ''}; public saving = false; public saveMessage = ''; + /** Keeps the confirmation on screen for a moment after the bar would otherwise go. */ + public justSaved = false; public saveError = ''; public get customPronouns(): boolean { return this.formPronouns.pronouns === '__customPronouns'; @@ -90,6 +96,16 @@ export class EditProfileFormComponent implements OnInit, OnDestroy { this.user.displayPeerProgress = true; } + // The values Discard puts back. Retaken after every successful save. + this.takeSnapshot(); + + // The same for Unit Hub updates, which are on in the bell and off everywhere + // else until the user opts in. + this.user.receiveUnitHubNotifications ??= true; + this.user.receiveUnitHubEmailNotifications ??= false; + this.user.receiveUnitHubPushNotifications ??= false; + this.user.receiveUnitHubSessionReminders ??= false; + if (!this.user.hasRunFirstTimeSetup) { this.user.optInToResearch = false; this.user.receiveFeedbackNotifications = true; @@ -101,6 +117,19 @@ export class EditProfileFormComponent implements OnInit, OnDestroy { ngOnDestroy(): void { this.pushSubscription?.unsubscribe(); + + // Clearing the timer here is not enough on its own. The save response can + // land after the view has gone, and the handler confirms the save, which + // arms a fresh timer against a component nothing will destroy again. The + // handlers check this flag instead of the request being cancelled here: + // unsubscribing would abort the PUT, so closing the dialog straight after + // pressing Save would silently lose the save. + this.destroyed = true; + + if (this.justSavedTimer) { + clearTimeout(this.justSavedTimer); + this.justSavedTimer = null; + } } /** @@ -179,6 +208,13 @@ export class EditProfileFormComponent implements OnInit, OnDestroy { this.authService.signOut(); } + /** The name shown in the profile header: preferred or first name, then last name. */ + public get displayName(): string { + const first = this.user?.nickname?.trim() || this.user?.firstName?.trim() || ''; + const name = [first, this.user?.lastName?.trim()].filter(Boolean).join(' '); + return name || this.user?.username || ''; + } + public get newUser(): boolean { return this.mode === 'new'; } @@ -193,11 +229,55 @@ export class EditProfileFormComponent implements OnInit, OnDestroy { } public get canEditEmail(): boolean { - return this.newUser || this.user.emailEditable === true; + return this.newUser || (this.user.emailEditable === true && !this.identityManaged); + } + + /** + * First and last name are asserted by the institution on every deployment + * that is not local database auth, so the API rejects a change to either. + * `institutionalIdentityManaged` is the server's own answer to that question, + * carried on the user we already fetched, so the page needs no extra request. + */ + public get canEditName(): boolean { + return this.newUser || !this.identityManaged; + } + + /** + * The server decides this, and a local database-auth demo always answers no. A dev + * build accepts ?identityManaged=1 so the institution-managed page can be seen + * without an SSO deployment. It changes nothing that is saved. + */ + public get identityManagedView(): boolean { + return this.identityManaged; + } + + private get identityManaged(): boolean { + if (this.user?.institutionalIdentityManaged) { + return true; + } + return ( + isDevMode() && new URLSearchParams(window.location.search).get('identityManaged') === '1' + ); + } + + /** + * Identity the deployment manages. These are rendered as facts rather than + * inputs, so they are left out of the update as well. Sending a value the + * user was never able to change is at best noise and at worst a 422. + */ + private get readOnlyIdentityKeys(): string[] { + const keys: string[] = []; + if (!this.canEditName) { + keys.push('firstName', 'lastName'); + } + if (!this.canEditEmail) { + keys.push('email'); + } + return keys; } public get canEditStudentId(): boolean { - return this.newUser || (!this.user.institutionalIdentityManaged && !this.managingOwnProfile); + return this.newUser || (!this.identityManaged && !this.managingOwnProfile); } public get canEditSystemRole(): boolean { @@ -215,6 +295,87 @@ export class EditProfileFormComponent implements OnInit, OnDestroy { return this.constants.IsTiiEnabled.value; } + /** The fields this form edits, for taking a snapshot to discard back to. */ + private static readonly EDITED_FIELDS = [ + 'firstName', + 'lastName', + 'nickname', + 'email', + 'studentId', + 'pronouns', + 'username', + 'systemRole', + 'optInToResearch', + 'displayPeerProgress', + 'acceptedTiiEula', + ] as const; + + private savedSnapshot: Record = {}; + /** The pronouns select sits outside the user object, so it is snapshotted too. */ + private savedFormPronouns = ''; + private justSavedTimer: ReturnType | null = null; + /** Set once the view has gone, so a late save response cannot touch it. */ + private destroyed = false; + + /** The user as it was when the form was last clean. */ + private takeSnapshot(): void { + const snapshot: Record = {}; + EditProfileFormComponent.EDITED_FIELDS.forEach((field) => { + snapshot[field] = (this.user as unknown as Record)[field]; + }); + this.savedSnapshot = snapshot; + this.savedFormPronouns = this.formPronouns.pronouns; + } + + /** + * In edit mode the bar earns its place only when it has something to say: an + * edit to save, a save in flight, a problem, or a confirmation that has not + * faded yet. The other modes always need their main action on screen. + * + * Dirtiness comes from the template's own form reference, so this does not + * depend on how the form directive happens to be resolved. + */ + public showActions(dirty: boolean): boolean { + if (this.mode !== 'edit') { + return true; + } + return dirty || this.saving || this.justSaved || !!this.saveError; + } + + /** Puts every edited field back to how it was when the form was last clean. */ + public discard(form?: NgForm): void { + if (this.saving) { + return; + } + + const target = this.user as unknown as Record; + Object.entries(this.savedSnapshot).forEach(([field, value]) => { + target[field] = value; + }); + + this.formPronouns.pronouns = this.savedFormPronouns; + this.saveMessage = ''; + this.saveError = ''; + form?.form.markAsPristine(); + form?.form.markAsUntouched(); + } + + private confirmSaved(message: string, form?: NgForm): void { + this.saveMessage = message; + form?.form.markAsPristine(); + this.takeSnapshot(); + + // The bar leaves on its own once the confirmation has been seen. + this.justSaved = true; + if (this.justSavedTimer) { + clearTimeout(this.justSavedTimer); + } + this.justSavedTimer = setTimeout(() => { + this.justSaved = false; + this.justSavedTimer = null; + }, SAVED_CONFIRMATION_MS); + } + public submit(form?: NgForm): void { if (this.saving || form?.invalid) { return; @@ -229,11 +390,13 @@ export class EditProfileFormComponent implements OnInit, OnDestroy { if (this.newUser) { this.userService.create(this.user).subscribe({ next: (updatedUser) => { + if (this.destroyed) { + return; + } this.saving = false; this.user = updatedUser; this.initialFirstName = this.user.firstName; - form?.form.markAsPristine(); - this.saveMessage = 'User created.'; + this.confirmSaved('User created.', form); this._snackBar.open('User created', 'dismiss', { duration: 1500, @@ -244,16 +407,23 @@ export class EditProfileFormComponent implements OnInit, OnDestroy { error: (error: unknown) => this.handleSaveError(error), }); } else { - this.userService.update(this.user).subscribe({ + const ignoreKeys = this.readOnlyIdentityKeys; + const request = ignoreKeys.length + ? this.userService.update(this.user, {entity: this.user, ignoreKeys}) + : this.userService.update(this.user); + + request.subscribe({ next: (updatedUser) => { + if (this.destroyed) { + return; + } this.saving = false; if (this.mode === 'create') { this.router.navigateByUrl('/home'); } else { this.user = updatedUser; this.initialFirstName = this.user.firstName; - form?.form.markAsPristine(); - this.saveMessage = 'Profile saved.'; + this.confirmSaved('Profile saved.', form); // TODO: refactor into new alertService // this is a new snackbar alert test @@ -270,13 +440,20 @@ export class EditProfileFormComponent implements OnInit, OnDestroy { } private handleSaveError(error: unknown): void { - this.saving = false; const serverMessage = error instanceof HttpErrorResponse ? error.error?.error : null; const message = typeof error === 'string' ? error : serverMessage; - this.saveError = + const text = typeof message === 'string' && message.trim() ? message : 'Profile could not be saved. Check your connection and try again.'; - this.alerts.error(this.saveError, 6000); + + // The alert still goes up after the view has gone: a save that failed is + // worth saying wherever the user ended up, and the message says which one. + // Only the in-form state is skipped, because there is no form left to show it. + if (!this.destroyed) { + this.saving = false; + this.saveError = text; + } + this.alerts.error(text, 6000); } } diff --git a/src/app/common/file-uploader/file-uploader.component.html b/src/app/common/file-uploader/file-uploader.component.html index 226e074463..0a91c4c0cf 100644 --- a/src/app/common/file-uploader/file-uploader.component.html +++ b/src/app/common/file-uploader/file-uploader.component.html @@ -1,5 +1,5 @@ - - +
+
@if (!showUploader) { } -
- @if (showUploader && uploadingInfo === null && shownUploadZones.length) { -
- @for (upload of shownUploadZones; track upload) { - @if (!singleDropZone && showName) { -
+
+ @if (showUploader && uploadingInfo === null && renderedUploadZones.length) { +
+ @for (upload of renderedUploadZones; track upload) { + @if (!showSummaryColumn && showName) { +
{{ uploadZones.length === 1 ? '' : $index + 1 + ' - ' }} {{ upload.display.name }}
} - @if (singleDropZone && showName) { -
Select {{ upload.display.name }}
+ @if (showSummaryColumn && showName) { +
{{ upload.display.name }}
}
@@ -66,10 +72,19 @@
Select {{ upload.display.name }}
/>
- @if (!singleDropZone && upload.model?.length > 0) { -
- - {{ upload.model[0].name }} + @if (!showSummaryColumn && upload.model?.length > 0) { +
+ + + {{ upload.model[0].name }} + {{ formatSize(upload.model[0].size) }} +
} - @if (showUploader && singleDropZone && uploadingInfo === null) { -
-
Upload Summary
- - @for (upload of uploadZones; track upload) { -
-
- - {{ upload.display.name }} -
+ @if (showUploader && showSummaryColumn && uploadingInfo === null) { +
+
Selected files
+
    + @for (upload of uploadZones; track upload) { @if (upload.model?.length > 0) { - {{ upload.model[0].name }} - +
  • + + + {{ upload.model[0].name }} + {{ upload.display.name }} · + {{ formatSize(upload.model[0].size) }} + + Ready + +
    • } @else { - File pending +
  • + + + {{ upload.display.name }} + {{ acceptedFormatsLabel(upload) }} + + @if (upload.display.error) { + Wrong type + } @else { + Waiting for file + } +
    • } -
- } + } +
}
@@ -137,54 +176,77 @@
Upload Summary
} @if (showUploader && readyToUpload() && isUploading) { - @if (!uploadingInfo?.complete) { -
-
- @for (upload of uploadZones; track upload) { - {{ upload.display.icon }} - } - arrow_right_alt - + + @if (showProgressPanel) { +
+ + +

{{ uploadTitle }}

+

{{ uploadingFileLabel }}

+ +
+
- - - + @if (!uploadSettling && !uploadComplete) { +

{{ uploadProgress }}%

+ + }
} - @if (uploadingInfo?.complete) { -
-
- - {{ uploadingInfo.success ? 'check_circle' : 'cancel' }} - - - - File Upload {{ uploadingInfo.success ? 'Successful' : 'Failed' }} - + + @if (uploadingInfo?.complete && !uploadingInfo.success) { +
+ +

Upload failed

+

{{ uploadingInfo.error }}

+ +
+ +
- - @if (!uploadingInfo.success) { -
-
-

Error Message: {{ uploadingInfo.error }}

- -
- - -
-
- }
} } - - +
+
diff --git a/src/app/common/file-uploader/file-uploader.component.scss b/src/app/common/file-uploader/file-uploader.component.scss index 1e45ae42da..55e069e72e 100644 --- a/src/app/common/file-uploader/file-uploader.component.scss +++ b/src/app/common/file-uploader/file-uploader.component.scss @@ -1,4 +1,3 @@ -.file-drop-zone .mat-icon, .complete .mat-icon { font-size: 50px; height: 50px; @@ -11,82 +10,268 @@ width: 75px; } +.file-input { + display: none; +} + +.file-uploader-card, +.file-uploader-content { + box-sizing: border-box; + min-width: 0; + width: 100%; +} + +// Sized by the space the uploader gets, not the viewport, so a wide dialog puts the +// drop zone and the file list side by side and a narrow one stacks them. +.file-uploader-content { + container-type: inline-size; +} + +.file-uploader-layout { + display: grid; + gap: 16px; + grid-template-columns: minmax(0, 1fr); + min-width: 0; +} + +@container (min-width: 600px) { + .file-uploader-layout--split { + grid-template-columns: repeat(2, minmax(0, 1fr)); + + > :only-child { + grid-column: 1 / -1; + } + } +} + +.file-uploader-zones, +.file-uploader-summary { + display: flex; + flex-direction: column; + gap: 8px; + min-width: 0; +} + +// A normal section label above each zone and above the file list. +.file-uploader-label { + margin: 0; + color: var(--ot-color-text); + font-size: 0.9rem; + font-weight: 600; + line-height: 1.4; +} + +// --- Drop zone --- + .file-drop-zone { align-items: center; - background: transparent; - border: 2px dashed var(--ot-color-border); - border-radius: var(--ot-radius-sm); - color: inherit; + background: var(--ot-color-surface); + border: 1.5px dashed var(--ot-color-border); + border-radius: var(--ot-radius-lg); + box-sizing: border-box; + color: var(--ot-color-text); cursor: pointer; display: flex; flex-direction: column; font: inherit; + gap: 4px; justify-content: center; - padding: 2.5rem; + min-height: 180px; + padding: 20px 16px; text-align: center; + width: 100%; + + // Children never take the drag events, so dragleave only fires on leaving the zone. + > * { + pointer-events: none; + } +} + +.file-drop-zone:hover { + border-color: var(--ot-color-link); } -.file-drop-zone:hover, .file-drop-zone:focus-visible { - border-color: currentColor; - outline: none; + border-color: var(--ot-color-link); + outline: 2px solid var(--ot-color-focus); + outline-offset: 2px; } -.file-input { - display: none; +.file-drop-zone--over { + background: color-mix(in srgb, var(--ot-color-link) 7%, var(--ot-color-surface)); + border-color: var(--ot-color-link); + border-style: solid; } -.file-uploader-card, -.file-uploader-content { - min-width: 0; - width: 100%; +.file-drop-zone--error, +.file-drop-zone--error:hover { + border-color: var(--ot-color-error); + border-style: solid; } -.selected-upload { +.file-drop-zone__icon { align-items: center; - display: grid; - gap: 0.75rem; - grid-template-columns: auto minmax(0, 1fr) 48px; - min-width: 0; + background: color-mix(in srgb, var(--ot-color-link) 12%, transparent); + border-radius: var(--ot-radius-circle); + color: var(--ot-color-link); + display: flex; + height: 48px; + justify-content: center; + margin-bottom: 6px; + transition: transform 150ms cubic-bezier(0.23, 1, 0.32, 1); + width: 48px; + + .mat-icon { + font-size: 26px; + height: 26px; + width: 26px; + } } -.selected-upload__name { - min-width: 0; +.file-drop-zone:hover .file-drop-zone__icon, +.file-drop-zone--over .file-drop-zone__icon { + transform: translateY(-2px); +} + +.file-drop-zone__icon--error { + background: color-mix(in srgb, var(--ot-color-error) 12%, transparent); + color: var(--ot-color-error); +} + +.file-drop-zone--error:hover .file-drop-zone__icon { + transform: none; +} + +.file-drop-zone__title { + font-size: 0.95rem; + font-weight: 600; + line-height: 1.4; +} + +.file-drop-zone__error { + color: color-mix(in srgb, var(--ot-color-error) 80%, var(--ot-color-text)); +} + +// Reads as a link: the whole zone is the button, so this is its visible call to action. +.file-drop-zone__browse { + color: var(--ot-color-link); + font-size: 0.875rem; + font-weight: 600; + text-decoration: underline; + text-underline-offset: 3px; +} + +.file-drop-zone__formats { + color: var(--ot-color-text-muted); + font-size: 0.78rem; + line-height: 1.4; + margin-top: 4px; overflow-wrap: anywhere; - word-break: break-word; } -.upload-summary-row { +@media (prefers-reduced-motion: reduce) { + .file-drop-zone__icon { + transition: none; + } + + .file-drop-zone:hover .file-drop-zone__icon, + .file-drop-zone--over .file-drop-zone__icon { + transform: none; + } +} + +// --- Selected files --- + +.upload-file-list { + display: flex; + flex-direction: column; + gap: 8px; + list-style: none; + margin: 0; + padding: 0; +} + +.upload-file-row { align-items: center; - display: grid; - gap: 0.5rem; - grid-template-columns: minmax(0, 1fr) minmax(0, 2fr) 48px; + background: var(--ot-color-surface); + border: 1px solid color-mix(in srgb, var(--ot-color-border) 40%, transparent); + border-radius: var(--ot-radius-md); + box-sizing: border-box; + display: flex; + gap: 10px; + min-height: 56px; min-width: 0; + padding: 6px 6px 6px 12px; } -.upload-summary-row__label { - align-items: center; +.upload-file-row--placeholder { + background: transparent; + border-style: dashed; + padding-right: 12px; +} + +.upload-file-row__icon { + color: var(--ot-color-link); + flex-shrink: 0; +} + +.upload-file-row--placeholder .upload-file-row__icon { + color: var(--ot-color-text-muted); +} + +.upload-file-row__text { display: flex; - gap: 0.25rem; + flex: 1 1 auto; + flex-direction: column; min-width: 0; } -@media (max-width: 639.98px) { - .file-uploader-content { - padding-inline: 0.75rem; - } +.upload-file-row__name { + font-weight: 600; + line-height: 1.4; + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; +} - .file-drop-zone { - min-height: clamp(12rem, 50dvh, 18rem); - padding: 1.25rem; - } +.upload-file-row__meta { + color: var(--ot-color-text-muted); + font-size: 0.78rem; + line-height: 1.4; + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; +} - .upload-summary-row { - grid-template-columns: minmax(0, 1fr) 48px; - } +.upload-chip { + background: color-mix(in srgb, var(--ot-color-text) 6%, transparent); + border: 1px solid color-mix(in srgb, var(--ot-color-border) 45%, transparent); + border-radius: var(--ot-radius-pill); + color: var(--ot-color-text-muted); + flex-shrink: 0; + font-size: 0.75rem; + font-weight: 600; + line-height: 1.5; + padding: 1px 10px; + white-space: nowrap; +} - .upload-summary-row__label { - grid-column: 1 / -1; +.upload-chip--success { + background: color-mix(in srgb, var(--ot-color-success) 12%, transparent); + border-color: color-mix(in srgb, var(--ot-color-success) 40%, transparent); + color: color-mix(in srgb, var(--ot-color-success) 80%, var(--ot-color-text)); +} + +.upload-chip--error { + background: color-mix(in srgb, var(--ot-color-error) 12%, transparent); + border-color: color-mix(in srgb, var(--ot-color-error) 40%, transparent); + color: color-mix(in srgb, var(--ot-color-error) 80%, var(--ot-color-text)); +} + +@media (max-width: 639.98px) { + .file-drop-zone { + min-height: 160px; + padding: 16px 12px; } .uploading, @@ -108,3 +293,249 @@ overflow-wrap: anywhere; } } + +// --- Uploading --- +// One panel, the same elements from sending to sent: the circle greens when the +// bytes land, the bar draws into the middle, the circle absorbs it, and the mark +// draws out of it. Smaller than the submit dialog's, because this one sits +// inside a card rather than taking over a surface. + +$upload-ease-out: cubic-bezier(0.23, 1, 0.32, 1); +$upload-ease-in-out: cubic-bezier(0.77, 0, 0.175, 1); + +.upload-flow, +.upload-outcome { + display: flex; + flex-direction: column; + align-items: center; + gap: 6px; + min-height: 220px; + padding: 28px 16px; + text-align: center; + animation: upload-panel-in 220ms $upload-ease-out both; +} + +// Holds the finished mark's height from the first frame, so the circle growing +// inside it never pushes the words below it down. +.upload-flow__mark { + display: inline-flex; + align-items: center; + justify-content: center; + height: 72px; +} + +.upload-flow__ring, +.upload-outcome__well { + position: relative; + display: inline-flex; + align-items: center; + justify-content: center; + width: 52px; + height: 52px; + border-radius: var(--ot-radius-circle, 999px); + background: color-mix(in srgb, var(--ot-color-primary) 14%, transparent); + color: var(--ot-color-primary); + + mat-icon { + width: 24px; + height: 24px; + font-size: 24px; + } +} + +.upload-flow__ring { + transition: + width 380ms $upload-ease-in-out, + height 380ms $upload-ease-in-out, + background-color 300ms ease, + color 300ms ease; +} + +.upload-flow__glyph { + transition: opacity 180ms ease; +} + +// Breathing while there is still something to wait for. Opacity only, so nothing +// is laid out again. +.upload-flow:not(.upload-flow--done) .upload-flow__ring { + animation: upload-breathe 1600ms ease-in-out infinite; +} + +.upload-flow__title, +.upload-outcome__title { + margin: 10px 0 0; + color: var(--ot-color-text); + font-size: 1rem; + font-weight: 620; + line-height: 1.3; + transition: opacity 180ms ease; +} + +.upload-flow__detail, +.upload-outcome__detail { + max-width: 38ch; + margin: 0; + color: var(--ot-color-text-muted); + font-size: 0.85rem; + line-height: 1.45; + overflow-wrap: anywhere; + transition: opacity 180ms ease; +} + +.upload-flow__track { + position: relative; + overflow: hidden; + width: min(320px, 100%); + height: 8px; + margin-block-start: 12px; + border-radius: 999px; + background: var(--ot-color-surface-raised); + // Narrows into the middle, then folds its own height away. It never simply + // disappears: something vanishing is what reads as a new screen. + transition: + width 300ms $upload-ease-in-out, + height 200ms $upload-ease-out 260ms, + margin 200ms $upload-ease-out 260ms, + opacity 160ms ease 280ms; +} + +.upload-flow__fill { + position: absolute; + inset: 0; + border-radius: inherit; + background: var(--ot-color-primary); + // Clipped rather than sized, so each progress event stays on the compositor. + clip-path: inset(0 calc(100% - var(--upload-progress, 0%)) 0 0); + transition: + clip-path 260ms $upload-ease-out, + background-color 300ms ease; +} + +.upload-flow__value { + margin: 0; + color: var(--ot-color-text-muted); + font-size: 0.78rem; + font-variant-numeric: tabular-nums; +} + +.upload-flow__action, +.upload-outcome button { + margin-block-start: 12px; + transition: transform 140ms $upload-ease-out; + + &:active { + transform: scale(0.97); + } +} + +// The bytes are away. +.upload-flow--done { + .upload-flow__ring { + background: color-mix(in srgb, var(--ot-color-success) 16%, transparent); + color: var(--ot-color-success); + } + + .upload-flow__fill { + background: var(--ot-color-success); + } + + .upload-flow__track { + width: 0; + } +} + +// Sent, and this component is the one saying so. +.upload-flow--complete { + .upload-flow__ring { + width: 68px; + height: 68px; + background: transparent; + animation: upload-absorb 320ms $upload-ease-out both; + } + + .upload-flow__track { + height: 0; + margin-block-start: 0; + opacity: 0; + } + + .upload-flow__title { + animation: upload-panel-in 280ms $upload-ease-out 200ms both; + } + + .upload-flow__detail { + animation: upload-panel-in 280ms $upload-ease-out 280ms both; + } +} + +.upload-outcome__well--error { + background: color-mix(in srgb, var(--ot-color-error) 16%, transparent); + color: var(--ot-color-error); +} + +.upload-outcome__actions { + display: flex; + flex-wrap: wrap; + justify-content: center; + gap: 10px; +} + +@keyframes upload-panel-in { + from { + opacity: 0; + transform: translateY(8px); + } + + to { + opacity: 1; + transform: none; + } +} + +@keyframes upload-absorb { + 0% { + transform: scale(1); + } + + 45% { + transform: scale(1.07); + } + + 100% { + transform: scale(1); + } +} + +@keyframes upload-breathe { + 0%, + 100% { + opacity: 1; + } + + 50% { + opacity: 0.55; + } +} + +@media (prefers-reduced-motion: reduce) { + .upload-flow, + .upload-outcome, + .upload-flow__ring, + .upload-flow__title, + .upload-flow__detail { + animation: none; + opacity: 1; + transform: none; + } + + .upload-flow__ring, + .upload-flow__glyph, + .upload-flow__title, + .upload-flow__detail, + .upload-flow__track, + .upload-flow__fill, + .upload-flow__action, + .upload-outcome button { + transition: none; + } +} diff --git a/src/app/common/file-uploader/file-uploader.component.spec.ts b/src/app/common/file-uploader/file-uploader.component.spec.ts index 07855faa53..7d8462d58d 100644 --- a/src/app/common/file-uploader/file-uploader.component.spec.ts +++ b/src/app/common/file-uploader/file-uploader.component.spec.ts @@ -42,7 +42,7 @@ describe('FileUploaderComponent responsive selected-file state', () => { fixture.detectChanges(); }); - it('wraps a long selected filename and exposes a named remove control', () => { + it('keeps a long selected filename readable and exposes a named remove control', () => { const file = new File( ['portfolio'], 'A very long learning summary report filename that must remain readable on a phone.pdf', @@ -59,6 +59,7 @@ describe('FileUploaderComponent responsive selected-file state', () => { '.selected-upload button', ) as HTMLButtonElement; expect(name.textContent).toContain(file.name); + expect(name.getAttribute('title')).toBe(file.name); expect(remove.getAttribute('aria-label')).toBe(`Remove ${file.name}`); expect(component.readyToUpload()).toBe(true); }); diff --git a/src/app/common/file-uploader/file-uploader.component.ts b/src/app/common/file-uploader/file-uploader.component.ts index c401561970..5ccdb07183 100644 --- a/src/app/common/file-uploader/file-uploader.component.ts +++ b/src/app/common/file-uploader/file-uploader.component.ts @@ -4,10 +4,12 @@ import { EventEmitter, Input, OnChanges, + OnDestroy, OnInit, Output, SimpleChanges, } from '@angular/core'; +import {Subscription} from 'rxjs'; import {UserService} from 'src/app/api/services/user.service'; import {DoubtfireConstants} from 'src/app/config/constants/doubtfire-constants'; import {ACCEPTED_TYPES} from './file-upload-types'; @@ -50,7 +52,7 @@ interface UploadingInfo { changeDetection: ChangeDetectionStrategy.Eager, standalone: false, }) -export class FileUploaderComponent implements OnInit, OnChanges { +export class FileUploaderComponent implements OnInit, OnChanges, OnDestroy { @Input() files: FileUploadSpec; @Input() url: string; @Input() method = 'POST'; @@ -69,8 +71,26 @@ export class FileUploaderComponent implements OnInit, OnChanges { @Input() showName: boolean = true; @Input() asButton: boolean = false; @Input() singleDropZone: boolean = false; + /** + * Set false when the host shows its own confirmation after a successful upload, + * so the two do not play one after the other. A failure still reports here, + * because only this component knows how to retry it. + */ + @Input() showSuccessState: boolean = true; + /** + * Set false when the host draws the in-flight state itself, so one panel can + * carry the upload all the way into its own confirmation instead of handing + * over to a second one. A failure still reports here. + */ + @Input() showProgressState: boolean = true; @Input() showUploadButton: boolean = true; @Input() resetAfterUpload: boolean = true; + /** + * What the in-flight panel calls what is being sent. This component is shared + * with the CSV importers and the group-set editor, where "your work" is a + * tutor's enrolment file and belongs to nobody. + */ + @Input() uploadingLabel: string = 'Uploading'; @Input() initiateUpload?: () => void; @@ -89,6 +109,8 @@ export class FileUploaderComponent implements OnInit, OnChanges { public shownUploadZones: UploadZone[] = []; public uploadZones: UploadZone[] = []; public dropSupported: boolean = true; + /** The zone a file is being dragged over, for the drag-over style only. */ + public dragOverZone: UploadZone | null = null; constructor( private userService: UserService, @@ -96,6 +118,8 @@ export class FileUploaderComponent implements OnInit, OnChanges { ) {} private externalName: string = 'OnTrack'; + private externalNameSub: Subscription | null = null; + private completionTimer: ReturnType | null = null; private activeRequest?: XMLHttpRequest; private uploadWasCancelled = false; @@ -111,11 +135,28 @@ export class FileUploaderComponent implements OnInit, OnChanges { this.resetUploader(); - this.constants.ExternalName.subscribe((name) => { + this.externalNameSub = this.constants.ExternalName.subscribe((name) => { this.externalName = name; }); } + ngOnDestroy(): void { + // ExternalName is a BehaviorSubject on a root service, so it never + // completes. Left subscribed, every dialog that has ever held an uploader + // stays in memory for the session. + this.externalNameSub?.unsubscribe(); + this.externalNameSub = null; + + // The completion callback runs on a delay, and the host it calls back into + // may be gone by then: closing the dialog in that window had a destroyed + // component apply the submission and claim its confirmation, so the student + // was told nothing at all. + if (this.completionTimer) { + clearTimeout(this.completionTimer); + this.completionTimer = null; + } + } + ngOnChanges(changes: SimpleChanges): void { if (changes['files']) { this.createUploadZones(changes.files.currentValue); @@ -128,19 +169,22 @@ export class FileUploaderComponent implements OnInit, OnChanges { this.uploadingInfo = null; } - public onDragOver(event: DragEvent) { + public onDragOver(event: DragEvent, upload?: UploadZone) { event.preventDefault(); event.stopPropagation(); + this.dragOverZone = upload ?? null; } public onDragLeave(event: DragEvent) { event.preventDefault(); event.stopPropagation(); + this.dragOverZone = null; } public onFileDropped(event: DragEvent, upload: UploadZone) { event.preventDefault(); event.stopPropagation(); + this.dragOverZone = null; const file = event.dataTransfer?.files?.[0]; if (file) { @@ -189,6 +233,88 @@ export class FileUploaderComponent implements OnInit, OnChanges { this.updateReadyState(this.readyToUpload()); } + /** + * The summary column beside the drop zone is a second copy of the same state. + * It earns its place only when there is more than one file to keep track of; + * with one, the zone becomes the selected file in place instead. + */ + public get showSummaryColumn(): boolean { + return this.singleDropZone && this.uploadZones.length > 1; + } + + /** + * With the summary column the left side narrows to the next zone still waiting + * for a file. Without it, every zone stays on screen and each one turns into + * its own selected file, so nothing disappears when a file is chosen. + */ + public get renderedUploadZones(): UploadZone[] { + return this.showSummaryColumn ? this.shownUploadZones : this.uploadZones; + } + + /** + * The request has landed but the host has not taken over yet. Completion fires + * `onComplete` on a short delay, and unmounting this panel the moment the bytes + * arrive left the dialog empty for that whole window. + */ + public get uploadSettling(): boolean { + return ( + !this.showSuccessState && + this.uploadingInfo?.complete === true && + this.uploadingInfo?.success === true + ); + } + + /** A success this component is showing itself, rather than handing over. */ + public get uploadComplete(): boolean { + return this.showSuccessState && this.uploadLanded; + } + + public get showProgressPanel(): boolean { + if (!this.showProgressState) { + return false; + } + return !this.uploadingInfo?.complete || this.uploadSettling || this.uploadComplete; + } + + public get uploadTitle(): string { + if (this.uploadComplete || this.uploadSettling) { + return 'Uploaded'; + } + return this.uploadingLabel; + } + + /** Percent sent so far, for a host drawing the in-flight state itself. */ + public get uploadProgress(): number { + return this.uploadingInfo?.progress ?? 0; + } + + /** True once the bytes are away, whether or not the host has taken over. */ + public get uploadLanded(): boolean { + return this.uploadingInfo?.complete === true && this.uploadingInfo?.success === true; + } + + /** + * Bytes actually on the wire. `isUploading` stays true after the request + * settles, because the panels that report the outcome are rendered under it, + * so a host asking "is there something here I would interrupt?" has to ask + * this instead. + */ + public get uploadInFlight(): boolean { + return this.isUploading && this.uploadingInfo?.complete !== true; + } + + /** The file being sent, or a count once there is more than one. */ + public get uploadingFileLabel(): string { + const named = this.uploadZones + .map((zone) => zone.model?.[0]?.name) + .filter((name): name is string => !!name); + + if (named.length === 0) { + return ''; + } + return named.length === 1 ? named[0] : `${named.length} files`; + } + readyToUpload(): boolean { return this.uploadZones.every((zone) => zone.model?.length); } @@ -287,7 +413,8 @@ export class FileUploaderComponent implements OnInit, OnChanges { if (xhr.status >= 200 && xhr.status < 300) { this.onSuccess?.(response); this.uploadingInfo.success = true; - setTimeout(() => { + this.completionTimer = setTimeout(() => { + this.completionTimer = null; this.onComplete?.(); if (this.resetAfterUpload) { this.resetUploader(); @@ -323,6 +450,45 @@ export class FileUploaderComponent implements OnInit, OnChanges { this.onCancelUpload?.(); } + /** What the drop zone asks for, e.g. "PDF" or "code file". */ + public dropNoun(upload: UploadZone): string { + const type = upload.display.type; + if (type === 'PDF' || type === 'image') { + return type; + } + return `${type === 'zip' ? 'ZIP' : type} file`; + } + + /** The accepted formats in words, e.g. "PDF or PS", shortened when the list is long. */ + public acceptedFormatsLabel(upload: UploadZone): string { + const formats = upload.accepts.map((ext) => ext.toUpperCase()); + const previewLimit = 4; + if (formats.length > previewLimit) { + const rest = formats.length - previewLimit; + return `${formats.slice(0, previewLimit).join(', ')} and ${rest} more`; + } + if (formats.length <= 1) { + return formats.join(''); + } + return `${formats.slice(0, -1).join(', ')} or ${formats[formats.length - 1]}`; + } + + /** A file size in words, e.g. "1.2 MB". */ + public formatSize(bytes: number | undefined): string { + if (bytes == null || !Number.isFinite(bytes)) { + return ''; + } + if (bytes < 1024) { + return `${bytes} B`; + } + const kb = bytes / 1024; + if (kb < 1024) { + return `${kb.toFixed(kb < 10 ? 1 : 0)} KB`; + } + const mb = kb / 1024; + return `${mb.toFixed(mb < 10 ? 1 : 0)} MB`; + } + // onClickFailureCancelInternal() { // console.log('onClickFailureCancelInternal'); // } diff --git a/src/app/common/header/notification-bell/notification-bell.component.scss b/src/app/common/header/notification-bell/notification-bell.component.scss index 1fc6d4dc86..493f6562c7 100644 --- a/src/app/common/header/notification-bell/notification-bell.component.scss +++ b/src/app/common/header/notification-bell/notification-bell.component.scss @@ -191,6 +191,13 @@ background: var(--ot-color-hover); color: var(--ot-color-text-muted); } + + // Info is already portfolio, so the Unit Hub takes the selected pair, which + // the theme keeps readable against each other in light and dark. + &.tone-unit-hub { + background: var(--ot-color-selected); + color: var(--ot-color-selected-text); + } } .notification-message { diff --git a/src/app/common/modals/about-doubtfire-modal/about-doubtfire-modal-content.component.html b/src/app/common/modals/about-doubtfire-modal/about-doubtfire-modal-content.component.html index 5409fa2312..3a9ae7da93 100644 --- a/src/app/common/modals/about-doubtfire-modal/about-doubtfire-modal-content.component.html +++ b/src/app/common/modals/about-doubtfire-modal/about-doubtfire-modal-content.component.html @@ -1,101 +1,159 @@ - -
- OnTrack logo -

+
+ +
+

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

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

+

Task-based learning, feedback and portfolio assessment.

- -
-

Lead Contributors

-
    - @for (person of data.mainContributors; track person) { -
  • - - -
    {{ person.name }}
    -
    @{{ person.login }}
    -     + + + +
    +

    + + Lead contributors +

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

    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! -

    -
+
+

+ + 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.component.ts b/src/app/common/modals/about-doubtfire-modal/about-doubtfire-modal.component.ts index 123accbbb9..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 @@ -12,6 +12,7 @@ import {GithubProfile} from './github-profile'; @Component({ selector: 'about-doubtfire-dialog', templateUrl: 'about-doubtfire-modal-content.component.html', + styleUrls: ['about-doubtfire-modal.scss'], changeDetection: ChangeDetectionStrategy.Eager, standalone: false, }) 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..d70ac1979e 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,390 @@ +// 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); +} + +.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/calendar-modal/calendar-modal.component.scss b/src/app/common/modals/calendar-modal/calendar-modal.component.scss index fe71ebf7aa..e6c60deacc 100644 --- a/src/app/common/modals/calendar-modal/calendar-modal.component.scss +++ b/src/app/common/modals/calendar-modal/calendar-modal.component.scss @@ -15,7 +15,10 @@ } .calendar-dialog__header h2 { + // mat-dialog-title carries Material's own 24px side padding, which pushed the + // title in from the eyebrow above it. The header already sets the inset. margin: 0; + padding: 0; line-height: 1.25; } diff --git a/src/app/common/modals/extension-modal/extension-modal.component.html b/src/app/common/modals/extension-modal/extension-modal.component.html index c304d2872d..7940d3e922 100644 --- a/src/app/common/modals/extension-modal/extension-modal.component.html +++ b/src/app/common/modals/extension-modal/extension-modal.component.html @@ -1,22 +1,51 @@ -

Request extension

+

Request an extension

-

- Please explain why you require an extension for this task, the teaching team will assess the - request shortly. +

+ @if (data.task.definition) { + + {{ data.task.definition.abbreviation }} + {{ data.task.definition.name }} + + } + + + Due {{ formatShortDate(dueDate) }} + +
+ + @if (daysPastDue > 0) { +

+ + This task is {{ daysPastDue }} {{ daysPastDue === 1 ? 'day' : 'days' }} past its due + date +

+ } + +

+ Tell your teaching team why you need more time. They'll review your request and reply in the + task comments.

- + Reason - {{ extensionData.controls.extensionReason.value.length }} / {{ reasonMaxLength }}{{ reasonLength }} / {{ reasonMaxLength }} @if (extensionData.controls.extensionReason.hasError('required')) { You must enter a reason @@ -29,40 +58,68 @@

Request extension

} - - Choose a date - - - - +
+ + New due date + + + + + @if (dateRangeText) { + Choose a date from {{ dateRangeText }} + } @else if (!hasDateRange) { + The final deadline has passed, so the earliest date is requested + } @else { + + Choose a date on or after {{ formatShortDate(minDate) }} + } + + + @if (dateNeedsAttention) { +

{{ dateErrorText }}

+ } @else if (extensionSummary) { + {{ extensionSummary }} + } +
@if (errorMessage) { }
-
- +
+
diff --git a/src/app/common/modals/extension-modal/extension-modal.component.scss b/src/app/common/modals/extension-modal/extension-modal.component.scss index ed62ef2a07..f02ef2f2d4 100644 --- a/src/app/common/modals/extension-modal/extension-modal.component.scss +++ b/src/app/common/modals/extension-modal/extension-modal.component.scss @@ -1,29 +1,162 @@ +@use '../../../unit-hub/hub-card' as hub; + +@include hub.card-theme; + :host { display: block; max-width: 100%; } +.ext-dialog-title { + padding-inline: 24px; +} + .task-dialog-content { display: flex; max-height: calc(100dvh - 10rem); flex-direction: column; - gap: 0.5rem; + gap: 16px; overflow: auto; + padding: 0 24px 8px; + color: var(--ot-color-text); +} + +// Task abbreviation, name and current due date under the title. +.ext-context { + display: flex; + flex-wrap: wrap; + align-items: center; + gap: 8px 16px; + margin-top: -4px; + font-size: 0.875rem; + line-height: 1.4; +} + +.ext-context__task { + display: inline-flex; + min-width: 0; + align-items: center; + gap: 8px; +} + +.ext-context__name { + overflow-wrap: anywhere; + font-weight: 600; +} + +.ext-context__due { + display: inline-flex; + align-items: center; + gap: 4px; + color: var(--ot-color-text-muted); + white-space: nowrap; + + mat-icon { + width: 18px; + height: 18px; + font-size: 18px; + color: var(--ot-color-link); + } +} + +// Same chip as the extension request card in the task comments. +.ext-chip { + display: inline-flex; + align-items: center; + gap: 4px; + padding: 2px 8px; + border-radius: var(--ot-radius-pill); + background: color-mix(in srgb, var(--ot-color-primary) 14%, transparent); + color: var(--ot-color-text); + font-size: 0.75rem; + font-weight: 500; + line-height: 1.4; + white-space: nowrap; +} + +.ext-chip--task { + font-weight: 600; +} + +.ext-callout { + @include hub.callout(raised); + @include hub.tinted-card(var(--ot-color-warning)); + margin: 0; + color: var(--ot-color-text); + + > mat-icon { + color: var(--ot-color-warning); + } +} + +.ext-description { + margin: 0; + color: var(--ot-color-text-muted); + line-height: 1.5; +} + +// The hint row sits inside the field, so no extra spacing is added below it. +.ext-field { + margin: 0; +} + +.ext-counter { + color: var(--ot-color-text-muted); + font-variant-numeric: tabular-nums; +} + +.ext-counter--warn { + color: var(--ot-color-warning); +} + +.ext-counter--limit { + color: var(--ot-color-error); + font-weight: 600; +} + +.ext-date { + display: flex; + flex-direction: column; + align-items: flex-start; + gap: 8px; +} + +.ext-summary { + font-variant-numeric: tabular-nums; +} + +.ext-date__error { + margin: 0; + color: var(--ot-color-error); + font-size: 0.75rem; } .task-dialog-actions { display: flex; flex-wrap: wrap; justify-content: flex-end; - gap: 0.5rem; - padding: 0.75rem 1.5rem 1rem; + gap: 12px; + padding: 16px 24px 24px; button { min-height: 44px; + margin: 0; white-space: normal; } } +// The app paints every spinner circle in the divider colour, which vanishes on a +// disabled button. Inside the button it follows the label colour instead. +.ext-submit__spinner { + display: inline-block; + margin-right: 8px; + vertical-align: middle; + + ::ng-deep circle { + stroke: currentColor !important; + } +} + .task-dialog-error { margin: 0; border-left: 4px solid var(--ot-color-error); @@ -32,14 +165,20 @@ color: var(--ot-color-text); } -@media (max-width: 479.98px) { +@media (max-width: 599.98px) { .task-dialog-content { max-height: calc(100dvh - 11.5rem); - padding-inline: 1rem; + padding-inline: 16px; + } + + .ext-dialog-title { + padding-inline: 16px; } .task-dialog-actions { - padding-inline: 1rem; + flex-direction: column-reverse; + align-items: stretch; + padding-inline: 16px; button { width: 100%; diff --git a/src/app/common/modals/extension-modal/extension-modal.component.spec.ts b/src/app/common/modals/extension-modal/extension-modal.component.spec.ts index d84cda3aa6..b7e6c495f3 100644 --- a/src/app/common/modals/extension-modal/extension-modal.component.spec.ts +++ b/src/app/common/modals/extension-modal/extension-modal.component.spec.ts @@ -1,17 +1,37 @@ -import {afterEach, describe, expect, it, vi} from 'vitest'; -import {MatDialogRef} from '@angular/material/dialog'; +import {afterEach, beforeEach, describe, expect, it, vi} from 'vitest'; +import {LOCALE_ID} from '@angular/core'; +import {ComponentFixture, TestBed} from '@angular/core/testing'; +import {ReactiveFormsModule} from '@angular/forms'; +import {MatButtonModule} from '@angular/material/button'; +import {provideNativeDateAdapter} from '@angular/material/core'; +import {MatDatepickerInputEvent, MatDatepickerModule} from '@angular/material/datepicker'; +import {MAT_DIALOG_DATA, MatDialogModule, MatDialogRef} from '@angular/material/dialog'; +import {MatIconModule} from '@angular/material/icon'; +import {MatInputModule} from '@angular/material/input'; +import {MatProgressSpinnerModule} from '@angular/material/progress-spinner'; import {of, throwError} from 'rxjs'; import {Task, TaskCommentService} from 'src/app/api/models/doubtfire-model'; import {AlertService} from '../../services/alert.service'; import {ExtensionModalComponent} from './extension-modal.component'; -function buildComponent(requestExtension = vi.fn(() => of({}))) { +function buildTask( + dueDate = new Date('2026-09-01T00:00:00Z'), + deadlineDate = new Date('2026-10-01T00:00:00Z'), +): Task { + return { + definition: {abbreviation: '1.1P', name: 'Hello World'}, + localDueDate: () => dueDate, + localDeadlineDate: () => deadlineDate, + } as unknown as Task; +} + +function pickDate(component: ExtensionModalComponent, value: Date | null): void { + component.addEvent('input', {value} as MatDatepickerInputEvent); +} + +function buildComponent(requestExtension = vi.fn(() => of({})), task = buildTask()) { const close = vi.fn(); const afterApplication = vi.fn(); - const task = { - localDueDate: () => new Date('2026-09-01T00:00:00Z'), - localDeadlineDate: () => new Date('2026-10-01T00:00:00Z'), - } as Task; const alerts = {success: vi.fn(), error: vi.fn()}; const component = new ExtensionModalComponent( {close} as unknown as MatDialogRef, @@ -57,6 +77,9 @@ describe('ExtensionModalComponent', () => { failure.component.extensionData.controls.extensionReason.setValue( 'A sufficiently detailed reason for the request', ); + // Submit refuses anything the button would refuse, so the date has to be + // picked here as it would be on screen. minDate is always in range. + pickDate(failure.component, failure.component.minDate); failure.component.submitApplication(); @@ -73,6 +96,7 @@ describe('ExtensionModalComponent', () => { success.component.extensionData.controls.extensionReason.setValue( 'A sufficiently detailed reason for the request', ); + pickDate(success.component, success.component.minDate); success.component.submitApplication(); expect(success.requestExtension).toHaveBeenCalled(); @@ -95,3 +119,273 @@ describe('ExtensionModalComponent', () => { vi.useRealTimers(); }); }); + +// Local dates, so the weekday labels hold in any time zone. 11 Sep 2026 is a Friday. +const DUE = new Date(2026, 8, 11, 23, 59); +const DEADLINE = new Date(2026, 9, 1, 23, 59); +const REASON = 'I was unwell for three days and missed the lab'; + +describe('ExtensionModalComponent presentation', () => { + beforeEach(() => { + vi.useFakeTimers({toFake: ['Date']}); + vi.setSystemTime(new Date(2026, 8, 5, 12, 0)); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + it('turns the counter to warning at 230 characters and error at the limit', () => { + const {component} = buildComponent(undefined, buildTask(DUE, DEADLINE)); + const reason = component.extensionData.controls.extensionReason; + + reason.setValue('a'.repeat(229)); + expect(component.reasonCounterState).toBe('ok'); + reason.setValue('a'.repeat(230)); + expect(component.reasonCounterState).toBe('warn'); + reason.setValue('a'.repeat(256)); + expect(component.reasonCounterState).toBe('limit'); + }); + + it('summarises a picked date against the current due date in requested weeks', () => { + const {component} = buildComponent(undefined, buildTask(DUE, DEADLINE)); + + expect(component.extensionSummary).toBe(''); + + pickDate(component, new Date(2026, 8, 17)); + expect(component.extensionDays).toBe(6); + expect(component.extensionSummary).toBe('+1 week · Thu 17 Sep'); + + pickDate(component, new Date(2026, 8, 21)); + expect(component.extensionDays).toBe(10); + expect(component.extensionSummary).toBe('+2 weeks · Mon 21 Sep'); + }); + + it('shows the due date, the allowed range and how far past due the task is', () => { + const onTime = buildComponent(undefined, buildTask(DUE, DEADLINE)).component; + expect(onTime.formatShortDate(onTime.dueDate)).toBe('Fri 11 Sep'); + expect(onTime.dateRangeText).toBe('Sat 12 Sep to Thu 1 Oct'); + expect(onTime.daysPastDue).toBe(0); + + vi.setSystemTime(new Date(2026, 8, 20, 12, 0)); + const late = buildComponent(undefined, buildTask(DUE, DEADLINE)).component; + expect(late.daysPastDue).toBe(8); + }); + + it('requests the earliest date without a pick once the final deadline has passed', () => { + vi.setSystemTime(new Date(2026, 9, 3, 12, 0)); + const {component} = buildComponent(undefined, buildTask(DUE, DEADLINE)); + + expect(component.hasDateRange).toBe(false); + expect(component.dateRangeText).toBe(''); + expect(component.extensionSummary).toBe('+4 weeks · Sun 4 Oct'); + + component.extensionData.controls.extensionReason.setValue(REASON); + expect(component.canSubmit).toBe(true); + }); + + it('refuses a date typed past the final deadline instead of sending it', () => { + // The out-of-range check used to return true outright once there was no + // range, so a typed 1 Jan 2030 went out as a 173 week request. + vi.setSystemTime(new Date(2026, 9, 3, 12, 0)); + const {component, requestExtension} = buildComponent(undefined, buildTask(DUE, DEADLINE)); + component.extensionData.controls.extensionReason.setValue(REASON); + + pickDate(component, new Date(2030, 0, 1)); + + // The date never lands, so nothing downstream can be derived from it. + expect(component.extensionSummary).toBe('+4 weeks · Sun 4 Oct'); + expect(component.isDateInRange).toBe(true); + expect(component.dateNeedsAttention).toBe(false); + + component.submitApplication(); + expect(requestExtension).toHaveBeenCalledWith(REASON, 4, expect.anything()); + }); + + it('holds the earliest date even if the guard is the only thing left', () => { + // Belt to the braces above: if the field is ever re-enabled, this is what + // stops a far-future date being requested. + vi.setSystemTime(new Date(2026, 9, 3, 12, 0)); + const {component} = buildComponent(undefined, buildTask(DUE, DEADLINE)); + + (component as unknown as {extensionDate: Date}).extensionDate = new Date(2030, 0, 1); + expect(component.isDateInRange).toBe(false); + expect(component.dateErrorText).toBe( + 'The final deadline has passed, so only the earliest date can be requested', + ); + }); + + it('never names a date range that resolved to nothing', () => { + // A task with no deadline leaves hasDateRange true and dateRangeText empty, + // which is the pair that used to render "Pick a date from ." and no hint. + const {component} = buildComponent(undefined, buildTask(DUE, null as unknown as Date)); + + expect(component.hasDateRange).toBe(true); + expect(component.dateRangeText).toBe(''); + expect(component.dateErrorText).toBe('Pick a date on or after Sat 12 Sep'); + }); + + it('keeps submit disabled until the reason and a date in range are both valid', () => { + const {component} = buildComponent(undefined, buildTask(DUE, DEADLINE)); + + expect(component.canSubmit).toBe(false); + component.extensionData.controls.extensionReason.setValue(REASON); + expect(component.canSubmit).toBe(false); + + pickDate(component, new Date(2026, 9, 20)); + expect(component.dateNeedsAttention).toBe(true); + expect(component.canSubmit).toBe(false); + + pickDate(component, null); + expect(component.canSubmit).toBe(false); + + pickDate(component, new Date(2026, 8, 17)); + expect(component.canSubmit).toBe(true); + + component.submitting = true; + expect(component.canSubmit).toBe(false); + }); +}); + +describe('ExtensionModalComponent template', () => { + let fixture: ComponentFixture; + let component: ExtensionModalComponent; + + beforeEach(async () => { + vi.useFakeTimers({toFake: ['Date']}); + vi.setSystemTime(new Date(2026, 8, 20, 12, 0)); + + await TestBed.configureTestingModule({ + declarations: [ExtensionModalComponent], + imports: [ + ReactiveFormsModule, + MatButtonModule, + MatDatepickerModule, + MatDialogModule, + MatIconModule, + MatInputModule, + MatProgressSpinnerModule, + ], + providers: [ + provideNativeDateAdapter(), + {provide: MatDialogRef, useValue: {close: vi.fn()}}, + {provide: MAT_DIALOG_DATA, useValue: {task: buildTask(DUE, DEADLINE)}}, + {provide: LOCALE_ID, useValue: 'en-US'}, + {provide: AlertService, useValue: {success: vi.fn(), error: vi.fn()}}, + {provide: TaskCommentService, useValue: {requestExtension: vi.fn(() => of({}))}}, + ], + }).compileComponents(); + + fixture = TestBed.createComponent(ExtensionModalComponent); + component = fixture.componentInstance; + fixture.detectChanges(); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + const el = () => fixture.nativeElement as HTMLElement; + const submitButton = () => el().querySelector('button.ext-submit'); + + it('renders the title, task context, past due callout and new copy', () => { + expect(el().querySelector('[mat-dialog-title]')?.textContent?.trim()).toBe( + 'Request an extension', + ); + expect(el().querySelector('.ext-context')?.textContent).toContain('1.1P'); + expect(el().querySelector('.ext-context')?.textContent).toContain('Hello World'); + expect(el().querySelector('.ext-context__due')?.textContent?.trim()).toContain( + 'Due Fri 11 Sep', + ); + expect(el().querySelector('.ext-callout')?.textContent?.replace(/\s+/g, ' ').trim()).toContain( + 'This task is 8 days past its due date', + ); + expect(el().querySelector('.ext-description')?.textContent?.replace(/\s+/g, ' ').trim()).toBe( + "Tell your teaching team why you need more time. They'll review your request and reply in the task comments.", + ); + }); + + it('labels the primary action without the date and enables it once valid', () => { + expect(submitButton()?.textContent?.trim()).toBe('send Request extension'); + expect(submitButton()?.disabled).toBe(true); + + component.extensionData.controls.extensionReason.setValue(REASON); + pickDate(component, new Date(2026, 8, 25)); + fixture.detectChanges(); + + expect(submitButton()?.disabled).toBe(false); + expect(el().querySelector('.ext-summary')?.textContent?.trim()).toBe('+2 weeks · Fri 25 Sep'); + }); + + it('marks the counter as a warning from 230 characters', () => { + const counter = () => el().querySelector('.ext-counter'); + component.extensionData.controls.extensionReason.setValue('a'.repeat(230)); + fixture.detectChanges(); + + expect(counter()?.textContent?.trim()).toBe('230 / 256'); + expect(counter()?.classList).toContain('ext-counter--warn'); + }); + + it('shows a spinner and disables both actions while submitting', () => { + component.submitting = true; + fixture.detectChanges(); + + expect(el().querySelector('.ext-submit__spinner')).not.toBeNull(); + const buttons = Array.from( + el().querySelectorAll('.task-dialog-actions button'), + ); + expect(buttons.every((button) => button.disabled)).toBe(true); + }); +}); + +describe('ExtensionModalComponent template past the final deadline', () => { + let fixture: ComponentFixture; + + beforeEach(async () => { + vi.useFakeTimers({toFake: ['Date']}); + vi.setSystemTime(new Date(2026, 9, 3, 12, 0)); + + await TestBed.configureTestingModule({ + declarations: [ExtensionModalComponent], + imports: [ + ReactiveFormsModule, + MatButtonModule, + MatDatepickerModule, + MatDialogModule, + MatIconModule, + MatInputModule, + MatProgressSpinnerModule, + ], + providers: [ + provideNativeDateAdapter(), + {provide: MatDialogRef, useValue: {close: vi.fn()}}, + {provide: MAT_DIALOG_DATA, useValue: {task: buildTask(DUE, DEADLINE)}}, + {provide: LOCALE_ID, useValue: 'en-US'}, + {provide: AlertService, useValue: {success: vi.fn(), error: vi.fn()}}, + {provide: TaskCommentService, useValue: {requestExtension: vi.fn(() => of({}))}}, + ], + }).compileComponents(); + + fixture = TestBed.createComponent(ExtensionModalComponent); + fixture.detectChanges(); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + it('closes the date field rather than leaving it open over an empty range', () => { + const el = fixture.nativeElement as HTMLElement; + const input = el.querySelector('.ext-date input'); + const toggle = el.querySelector('mat-datepicker-toggle button'); + + expect(fixture.componentInstance.hasDateRange).toBe(false); + expect(input?.disabled).toBe(true); + // Disabling the input carries the picker and its toggle, so there is no way + // in through the calendar either. + expect(toggle?.disabled).toBe(true); + expect(el.querySelector('.ext-date mat-hint')?.textContent).toContain( + 'The final deadline has passed', + ); + }); +}); diff --git a/src/app/common/modals/extension-modal/extension-modal.component.ts b/src/app/common/modals/extension-modal/extension-modal.component.ts index 40b6114183..a56747d48f 100644 --- a/src/app/common/modals/extension-modal/extension-modal.component.ts +++ b/src/app/common/modals/extension-modal/extension-modal.component.ts @@ -1,4 +1,15 @@ -import {addDays, differenceInDays, differenceInWeeks, isAfter} from 'date-fns'; +import { + addDays, + differenceInCalendarDays, + differenceInDays, + differenceInWeeks, + endOfDay, + format, + isAfter, + isBefore, + isSameDay, + startOfDay, +} from 'date-fns'; import {ChangeDetectionStrategy, Component, Inject, LOCALE_ID} from '@angular/core'; import {FormControl, FormGroup, FormGroupDirective, NgForm, Validators} from '@angular/forms'; import {ErrorStateMatcher} from '@angular/material/core'; @@ -25,10 +36,13 @@ export class ReasonErrorStateMatcher implements ErrorStateMatcher { export class ExtensionModalComponent { protected reasonMinLength: number = 15; protected reasonMaxLength: number = 256; + /** The counter turns to the warning colour from this many characters. */ + protected readonly reasonWarnLength = 230; submitting = false; errorMessage = ''; private allowClose = false; private dateChanged = false; + private dateEntryInvalid = false; constructor( public dialogRef: MatDialogRef, @Inject(MAT_DIALOG_DATA) public data: {task: Task; afterApplication?: () => void}, @@ -69,12 +83,130 @@ export class ExtensionModalComponent { maxDate = this.data.task.localDeadlineDate(); // deadline, hard deadline extensionDate = new Date(this.minDate); addEvent(type: string, event: MatDatepickerInputEvent) { + // With no range left the request carries the earliest date, so there is + // nothing here to change. Refusing the write makes that an invariant rather + // than something the validity getters have to keep catching, and it means + // extensionDuration can only ever be derived from a date that was allowed. + if (!this.hasDateRange) { + return; + } + + this.dateEntryInvalid = !event.value; if (event.value) { this.extensionDate = new Date(event.value); this.dateChanged = true; } } + /** Short date for the dialog, e.g. "Fri 11 Sep". */ + public formatShortDate(date: Date): string { + return format(date, 'EEE d MMM'); + } + + public get dueDate(): Date { + return this.data.task.localDueDate(); + } + + /** + * Whole days the task is past its due date, or 0 when it is not overdue. Rounds down, + * like the "Passed Due Date By" banner on the task page. + */ + public get daysPastDue(): number { + const diff = Date.now() - this.dueDate.getTime(); + return diff > 0 ? Math.floor(diff / (1000 * 3600 * 24)) : 0; + } + + public get reasonLength(): number { + return this.extensionData.controls.extensionReason.value?.length ?? 0; + } + + public get reasonCounterState(): 'ok' | 'warn' | 'limit' { + if (this.reasonLength >= this.reasonMaxLength) { + return 'limit'; + } + return this.reasonLength >= this.reasonWarnLength ? 'warn' : 'ok'; + } + + /** + * False once the final deadline has passed, when the earliest date is after the latest. + * The request then goes out for the earliest date, as it always has. + */ + public get hasDateRange(): boolean { + return !this.maxDate || !isAfter(startOfDay(this.minDate), this.maxDate); + } + + /** The allowed range for the new due date, e.g. "Sat 12 Sep to Thu 1 Oct". */ + public get dateRangeText(): string { + if (!this.minDate || !this.maxDate || !this.hasDateRange) { + return ''; + } + return `${this.formatShortDate(this.minDate)} to ${this.formatShortDate(this.maxDate)}`; + } + + /** Calendar days between the current due date and the picked date. */ + public get extensionDays(): number { + return differenceInCalendarDays(this.extensionDate, this.dueDate); + } + + public get isDateInRange(): boolean { + if (!this.hasDateRange) { + // There is no range left to choose from, so the earliest date is the only + // one the request can carry. Returning true here instead turned every + // check off: a typed date of 1 Jan 2030 passed validation and went out as + // a 173 week extension. + return isSameDay(this.extensionDate, this.minDate); + } + if (this.minDate && isBefore(this.extensionDate, startOfDay(this.minDate))) { + return false; + } + if (this.maxDate && isAfter(this.extensionDate, endOfDay(this.maxDate))) { + return false; + } + return true; + } + + /** True once the date field holds something that cannot be requested. */ + public get dateNeedsAttention(): boolean { + return this.dateEntryInvalid || (this.dateChanged && !this.isDateInRange); + } + + /** + * Why the date will not do. hasDateRange and dateRangeText do not empty on + * the same condition, so this has to ask about the text it is going to use + * rather than assume a range exists wherever hasDateRange is true. Naming a + * range that resolved to nothing is what produced "Pick a date from ." + */ + public get dateErrorText(): string { + if (!this.hasDateRange) { + return 'The final deadline has passed, so only the earliest date can be requested'; + } + if (!this.dateRangeText) { + return `Pick a date on or after ${this.formatShortDate(this.minDate)}`; + } + return `Pick a date from ${this.dateRangeText}`; + } + + /** + * The picked extension as it will be requested, e.g. "+1 week · Thu 17 Sep". + * Requests are made in whole weeks, so the length is shown in weeks. + */ + public get extensionSummary(): string { + if ((!this.dateChanged && this.hasDateRange) || this.dateNeedsAttention) { + return ''; + } + const weeks = this.extensionDuration; + return `+${weeks} ${weeks === 1 ? 'week' : 'weeks'} · ${this.formatShortDate(this.extensionDate)}`; + } + + public get canSubmit(): boolean { + return ( + this.extensionData.valid && + (this.dateChanged || !this.hasDateRange) && + !this.dateNeedsAttention && + !this.submitting + ); + } + public get isDirty(): boolean { return this.extensionData.dirty || this.dateChanged; } @@ -102,7 +234,10 @@ export class ExtensionModalComponent { } submitApplication(): void { - if (this.submitting || this.extensionData.invalid) { + // Checks what the button checks, rather than only the reason. The date was + // guarded by the disabled state alone, so anything that reached this method + // with a bad date sent it. + if (!this.canSubmit) { this.extensionData.markAllAsTouched(); return; } diff --git a/src/app/common/modals/extension-modal/extension-modal.service.spec.ts b/src/app/common/modals/extension-modal/extension-modal.service.spec.ts index 82a12ab162..fa4d4f1a49 100644 --- a/src/app/common/modals/extension-modal/extension-modal.service.spec.ts +++ b/src/app/common/modals/extension-modal/extension-modal.service.spec.ts @@ -17,7 +17,7 @@ describe('ExtensionModalService', () => { autoFocus: 'dialog', closeOnNavigation: true, maxHeight: 'calc(100dvh - 2rem)', - maxWidth: '700px', + maxWidth: '560px', restoreFocus: true, width: 'calc(100vw - 2rem)', }); diff --git a/src/app/common/modals/extension-modal/extension-modal.service.ts b/src/app/common/modals/extension-modal/extension-modal.service.ts index 497acf9e5b..73c51be7a4 100644 --- a/src/app/common/modals/extension-modal/extension-modal.service.ts +++ b/src/app/common/modals/extension-modal/extension-modal.service.ts @@ -16,7 +16,7 @@ export class ExtensionModalService { autoFocus: 'dialog', closeOnNavigation: true, maxHeight: 'calc(100dvh - 2rem)', - maxWidth: '700px', + maxWidth: '560px', panelClass: 'responsive-task-dialog', restoreFocus: true, width: 'calc(100vw - 2rem)', 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

- +
- + } - +
diff --git a/src/app/common/notification-settings/notification-settings.component.html b/src/app/common/notification-settings/notification-settings.component.html index f43cd193da..2225697bfa 100644 --- a/src/app/common/notification-settings/notification-settings.component.html +++ b/src/app/common/notification-settings/notification-settings.component.html @@ -1,26 +1,31 @@
-

Notification Settings

+
+

+ Notifications +

-

- Choose which notification categories you want to receive. -

- - @if (showNotificationsLink) { -

- You can view the notifications controlled by these preferences on the - Notifications page. +

+ Choose which notification categories you want to receive.

- } + + @if (showNotificationsLink) { +

+ You can view the notifications controlled by these preferences on the + Notifications page. +

+ } +
@@ -36,6 +41,7 @@

Notification Settings

@@ -46,10 +52,40 @@

Notification Settings

+ +
+ + Summary email + + + @for (option of digestOptions; track option.value) { + {{ option.label }} + } + + + + {{ digestHelp }} + +
+
@@ -59,4 +95,55 @@

Notification Settings

Portfolio processing and assessment updates.
+ +
+ + Unit Hub updates + + + New and changed announcements, and sessions that move or are cancelled. Shown here in OnTrack. + Choose below if you also want them sent to you. + + +
+ + Email + + + Push + + + Session reminders + +
+ + Push only reaches browsers where push notifications are turned on. Session reminders arrive 30 + minutes before a session starts. + +
diff --git a/src/app/common/notification-settings/notification-settings.component.scss b/src/app/common/notification-settings/notification-settings.component.scss index 1957141191..8d9f170a31 100644 --- a/src/app/common/notification-settings/notification-settings.component.scss +++ b/src/app/common/notification-settings/notification-settings.component.scss @@ -1,3 +1,7 @@ +@use '../edit-profile-form/profile-form' as profile; + +@include profile.theme; + :host { display: block; min-width: 0; @@ -5,45 +9,62 @@ } .notification-settings { - border: 1px solid var(--ot-color-border); - border-radius: 12px; - gap: 10px; - margin-block: 16px; - padding: 16px; - width: 100%; + @include profile.card; + gap: 20px; } -.notification-settings h3 { - color: var(--ot-color-text); - margin: 0; -} +.notification-settings__heading { + @include profile.card-heading; -.notification-settings__intro { - color: var(--ot-color-text-muted); - margin: 0; + a { + color: var(--ot-color-link); + text-decoration: underline; + text-underline-offset: 3px; + } } +// label and description on the left, the checkbox on the right; one gap between +// rows instead of a box around each .notification-setting { - border: 1px solid var(--ot-color-divider); - border-radius: 10px; + @include profile.setting-row; +} + +.notification-setting__channels { + display: flex; + flex-wrap: wrap; + gap: 0 16px; + margin-block: 4px 2px; + // line the boxes up with the text above, past the checkbox's touch padding + margin-inline-start: -11px; +} + +.notification-setting__channels mat-checkbox { + width: auto; +} + +// A choice rather than an on/off, so the control sits on the same line as its +// label the way the checkboxes do, with the help text underneath. +.notification-setting--choice { display: grid; - gap: 2px; - min-height: 64px; - padding: 8px 10px; + align-items: center; + gap: 4px 12px; + grid-template-areas: + 'label control' + 'help help'; + grid-template-columns: minmax(0, 1fr) auto; } -.notification-setting mat-checkbox { - width: 100%; +.notification-setting__label { + grid-area: label; + color: var(--ot-color-text); + font-size: 0.95rem; } -.notification-setting__help { - color: var(--ot-color-text-muted); - line-height: 1.4; - padding: 0 8px 4px 40px; +.notification-setting__select { + grid-area: control; + width: 10rem; } -@media (max-width: 420px) { - .notification-settings { - padding: 14px; - } +.notification-setting--choice .notification-setting__help { + grid-area: help; } diff --git a/src/app/common/notification-settings/notification-settings.component.spec.ts b/src/app/common/notification-settings/notification-settings.component.spec.ts index 64d67af027..2dc78852dc 100644 --- a/src/app/common/notification-settings/notification-settings.component.spec.ts +++ b/src/app/common/notification-settings/notification-settings.component.spec.ts @@ -4,6 +4,10 @@ import {ComponentFixture, TestBed} from '@angular/core/testing'; import {FormsModule} from '@angular/forms'; import {MatCheckboxModule} from '@angular/material/checkbox'; import {MatCheckboxHarness} from '@angular/material/checkbox/testing'; +import {MatFormFieldModule} from '@angular/material/form-field'; +import {MatIconModule} from '@angular/material/icon'; +import {MatSelectModule} from '@angular/material/select'; +import {NoopAnimationsModule} from '@angular/platform-browser/animations'; import {RouterModule} from '@angular/router'; import {User} from 'src/app/api/models/user/user'; import {NotificationSettingsComponent} from './notification-settings.component'; @@ -15,6 +19,10 @@ const makeUser = (): User => receiveTaskNotifications: false, receiveFeedbackNotifications: false, receivePortfolioNotifications: false, + receiveUnitHubNotifications: true, + receiveUnitHubEmailNotifications: false, + receiveUnitHubPushNotifications: false, + receiveUnitHubSessionReminders: false, }) as User; describe('NotificationSettingsComponent', () => { @@ -24,7 +32,15 @@ describe('NotificationSettingsComponent', () => { beforeEach(async () => { await TestBed.configureTestingModule({ declarations: [NotificationSettingsComponent], - imports: [FormsModule, MatCheckboxModule, RouterModule.forRoot([])], + imports: [ + FormsModule, + MatCheckboxModule, + MatFormFieldModule, + MatIconModule, + MatSelectModule, + NoopAnimationsModule, + RouterModule.forRoot([]), + ], }).compileComponents(); fixture = TestBed.createComponent(NotificationSettingsComponent); @@ -39,15 +55,72 @@ describe('NotificationSettingsComponent', () => { expect(component).toBeTruthy(); }); - it('shows the three notification preferences', async () => { + it('shows the four notification categories and the Unit Hub channels', async () => { const loader = TestbedHarnessEnvironment.loader(fixture); const checkboxes = await loader.getAllHarnesses(MatCheckboxHarness); - expect(checkboxes.length).toBe(3); + expect(await Promise.all(checkboxes.map((checkbox) => checkbox.getLabelText()))).toEqual([ + 'Task notifications', + 'Feedback notifications', + 'Portfolio notifications', + 'Unit Hub updates', + 'Email', + 'Push', + 'Session reminders', + ]); + }); + + it('renders the Unit Hub row like the other categories', () => { + const rows: HTMLElement[] = Array.from( + fixture.nativeElement.querySelectorAll( + '.notification-setting:not(.notification-setting--choice)', + ), + ); + const hubRow = rows[3]; - expect(await checkboxes[0].getLabelText()).toBe('Task notifications'); - expect(await checkboxes[1].getLabelText()).toBe('Feedback notifications'); - expect(await checkboxes[2].getLabelText()).toBe('Portfolio notifications'); + expect(rows.length).toBe(4); + expect(hubRow.querySelector('small')?.id).toBe('unit-hub-notification-description'); + expect(hubRow.textContent).toContain('New and changed announcements'); + expect(hubRow.querySelector('[role="group"]')?.getAttribute('aria-label')).toBe( + 'Also send Unit Hub updates by', + ); + }); + + it('starts Unit Hub updates in the app only, and saves each channel to its own field', async () => { + const loader = TestbedHarnessEnvironment.loader(fixture); + const hub = await loader.getHarness(MatCheckboxHarness.with({label: 'Unit Hub updates'})); + const email = await loader.getHarness(MatCheckboxHarness.with({label: 'Email'})); + const push = await loader.getHarness(MatCheckboxHarness.with({label: 'Push'})); + const reminders = await loader.getHarness( + MatCheckboxHarness.with({label: 'Session reminders'}), + ); + + expect(await hub.isChecked()).toBe(true); + expect(await email.isChecked()).toBe(false); + expect(await push.isChecked()).toBe(false); + expect(await reminders.isChecked()).toBe(false); + + await email.check(); + await reminders.check(); + + expect(component.user.receiveUnitHubEmailNotifications).toBe(true); + expect(component.user.receiveUnitHubPushNotifications).toBe(false); + expect(component.user.receiveUnitHubSessionReminders).toBe(true); + }); + + it('turns the Unit Hub channels off while the category is off', async () => { + const loader = TestbedHarnessEnvironment.loader(fixture); + const hub = await loader.getHarness(MatCheckboxHarness.with({label: 'Unit Hub updates'})); + + await hub.uncheck(); + fixture.detectChanges(); + await fixture.whenStable(); + + expect(component.user.receiveUnitHubNotifications).toBe(false); + for (const label of ['Email', 'Push', 'Session reminders']) { + const channel = await loader.getHarness(MatCheckboxHarness.with({label})); + expect(await channel.isDisabled()).toBe(true); + } }); it('updates the correct user preference when toggled', async () => { @@ -85,19 +158,25 @@ describe('NotificationSettingsComponent', () => { expect(text).toContain('help requests and extension requests'); expect(text).toContain('New comments, feedback, and review outcomes.'); expect(text).toContain('Portfolio processing and assessment updates.'); + expect(text).toContain('Session reminders arrive 30 minutes before a session starts.'); }); it('associates each checkbox with its help text', () => { - const inputs = fixture.nativeElement.querySelectorAll('input[type="checkbox"]'); - const descriptions = fixture.nativeElement.querySelectorAll('.notification-setting small'); + const inputs = fixture.nativeElement.querySelectorAll( + '.notification-setting > mat-checkbox input[type="checkbox"]', + ); + const descriptions = fixture.nativeElement.querySelectorAll( + '.notification-setting:not(.notification-setting--choice) small[id]', + ); const descriptionIds = [ 'task-notification-description', 'feedback-notification-description', 'portfolio-notification-description', + 'unit-hub-notification-description', ]; - expect(inputs.length).toBe(3); - expect(descriptions.length).toBe(3); + expect(inputs.length).toBe(4); + expect(descriptions.length).toBe(4); descriptionIds.forEach((id, index) => { expect(descriptions[index].id).toBe(id); @@ -122,4 +201,45 @@ describe('NotificationSettingsComponent', () => { expect(fixture.nativeElement.querySelector('a[href="/notifications"]')).toBeNull(); expect(fixture.nativeElement.textContent).not.toContain('Notifications page'); }); + + it('offers a cadence for the summary email and explains the one chosen', () => { + const component = fixture.componentInstance; + + expect(component.digestOptions.map((option) => option.value)).toEqual([ + 'off', + 'daily', + 'weekly', + 'monthly', + ]); + + component.user.digestFrequency = 'monthly'; + expect(component.digestHelp).toBe('How the trimester is going so far.'); + + // Never is a real choice here, separate from feedback notifications. + component.user.digestFrequency = 'off'; + expect(component.digestHelp).toBe('No summary email.'); + }); + + it('describes the cadence control the way the checkboxes are described', () => { + const select = fixture.nativeElement.querySelector('mat-select'); + const help = fixture.nativeElement.querySelector('#digest-frequency-description'); + + expect(select?.getAttribute('aria-describedby')).toBe('digest-frequency-description'); + expect(help).not.toBeNull(); + }); + + // label[for] cannot name the select, because Material puts role="combobox" on + // the select's own element rather than on a form control. + it('names the cadence control with the label beside it', () => { + const select = fixture.nativeElement.querySelector('mat-select'); + const label = fixture.nativeElement.querySelector('#digest-frequency-label'); + + expect(label?.textContent.trim()).toBe('Summary email'); + expect(select?.getAttribute('aria-labelledby')?.trim()).toBe('digest-frequency-label'); + }); + + it('says nothing rather than guessing when the cadence is unknown', () => { + fixture.componentInstance.user.digestFrequency = undefined as unknown as string; + expect(fixture.componentInstance.digestHelp).toBe(''); + }); }); diff --git a/src/app/common/notification-settings/notification-settings.component.ts b/src/app/common/notification-settings/notification-settings.component.ts index 734fec2c9c..022c09705c 100644 --- a/src/app/common/notification-settings/notification-settings.component.ts +++ b/src/app/common/notification-settings/notification-settings.component.ts @@ -16,4 +16,21 @@ export class NotificationSettingsComponent { * someone else, and on the first-login setup form. */ @Input() showNotificationsLink = false; + + /** + * The summary email's cadence. 'off' stops it without also turning off + * feedback notifications, which used to be the only switch it had. + */ + public readonly digestOptions: {value: string; label: string; help: string}[] = [ + {value: 'off', label: 'Never', help: 'No summary email.'}, + {value: 'daily', label: 'Daily', help: 'What is due next, every morning.'}, + {value: 'weekly', label: 'Weekly', help: 'Deadlines and how you are tracking.'}, + {value: 'monthly', label: 'Monthly', help: 'How the trimester is going so far.'}, + ]; + + public get digestHelp(): string { + return ( + this.digestOptions.find((option) => option.value === this.user?.digestFrequency)?.help ?? '' + ); + } } diff --git a/src/app/common/notifications-page/notifications-page.component.scss b/src/app/common/notifications-page/notifications-page.component.scss index 2731bf8e37..e2c4f31072 100644 --- a/src/app/common/notifications-page/notifications-page.component.scss +++ b/src/app/common/notifications-page/notifications-page.component.scss @@ -127,6 +127,13 @@ background: var(--ot-color-hover); color: var(--ot-color-text-muted); } + + // Info is already portfolio, so the Unit Hub takes the selected pair, which + // the theme keeps readable against each other in light and dark. + &.tone-unit-hub { + background: var(--ot-color-selected); + color: var(--ot-color-selected-text); + } } .notification-message { diff --git a/src/app/common/notifications/notification-open.service.spec.ts b/src/app/common/notifications/notification-open.service.spec.ts index 89adc8d275..dca61ee61b 100644 --- a/src/app/common/notifications/notification-open.service.spec.ts +++ b/src/app/common/notifications/notification-open.service.spec.ts @@ -151,6 +151,26 @@ describe('NotificationOpenService', () => { expect(alerts.error).toHaveBeenCalledWith(NOTIFICATION_UNAVAILABLE_MESSAGE); }); + it('opens a Unit Hub announcement on the hub without asking for a staff role', async () => { + users.currentUser = {id: 7, role: 'Tutor'}; + loading.next(true); + + const notification = fields({ + event: 'unit_announcement_published', + notificationType: 'unit_hub', + link: '/unit-hub?unit=3&announcement=12', + projectId: null, + studentId: null, + taskDefinitionAbbr: null, + announcementId: 12, + sessionId: null, + }); + + await expect(service.open(notification)).resolves.toBe(true); + + expect(routes.navigateToTarget).toHaveBeenCalledWith('/unit-hub?unit=3&announcement=12'); + }); + it('follows the link from an older api that sends no ids', async () => { const legacy = Object.assign(new Notification(), { event: 'task_comment_created', diff --git a/src/app/common/notifications/notification-presentation.spec.ts b/src/app/common/notifications/notification-presentation.spec.ts index ade886b4e3..e9af069cda 100644 --- a/src/app/common/notifications/notification-presentation.spec.ts +++ b/src/app/common/notifications/notification-presentation.spec.ts @@ -52,6 +52,27 @@ describe('notification presentation', () => { }); }); + it('draws Unit Hub announcements and sessions in their own tone', () => { + expect( + [ + 'unit_announcement_published', + 'unit_announcement_updated', + 'unit_session_changed', + 'unit_session_starting_soon', + ].map((event) => presentationFor(notification(event, 'unit_hub'))), + ).toEqual([ + {icon: 'campaign', label: 'Announcement', tone: 'unit-hub'}, + {icon: 'edit_note', label: 'Announcement updated', tone: 'unit-hub'}, + {icon: 'event_note', label: 'Session update', tone: 'unit-hub'}, + {icon: 'alarm', label: 'Starting soon', tone: 'unit-hub'}, + ]); + expect(presentationFor(notification('unit_hub_future_event', 'unit_hub'))).toEqual({ + icon: 'hub', + label: 'Unit Hub', + tone: 'unit-hub', + }); + }); + it('uses a category fallback for a new event and a generic fallback for a new category', () => { expect(presentationFor(notification('future_feedback_event', 'feedback'))).toMatchObject({ icon: 'chat_bubble', diff --git a/src/app/common/notifications/notification-presentation.ts b/src/app/common/notifications/notification-presentation.ts index dddd6d67f6..5d9cd4111d 100644 --- a/src/app/common/notifications/notification-presentation.ts +++ b/src/app/common/notifications/notification-presentation.ts @@ -3,7 +3,7 @@ import {Notification} from 'src/app/api/models/notification'; export interface NotificationPresentation { icon: string; label: string; - tone: 'feedback' | 'task' | 'portfolio' | 'extension' | 'general'; + tone: 'feedback' | 'task' | 'portfolio' | 'extension' | 'general' | 'unit-hub'; } const EVENT_PRESENTATIONS: Readonly> = { @@ -28,6 +28,11 @@ const EVENT_PRESENTATIONS: Readonly> = portfolio_submitted: {icon: 'inventory', label: 'Portfolio submitted', tone: 'portfolio'}, group_membership_changed: {icon: 'groups', label: 'Group update', tone: 'general'}, tutorial_changed: {icon: 'event_repeat', label: 'Tutorial update', tone: 'general'}, + // Unit Hub. A cancellation is a session change too, and says so in its message. + unit_announcement_published: {icon: 'campaign', label: 'Announcement', tone: 'unit-hub'}, + unit_announcement_updated: {icon: 'edit_note', label: 'Announcement updated', tone: 'unit-hub'}, + unit_session_changed: {icon: 'event_note', label: 'Session update', tone: 'unit-hub'}, + unit_session_starting_soon: {icon: 'alarm', label: 'Starting soon', tone: 'unit-hub'}, }; const CATEGORY_PRESENTATIONS: Readonly> = { @@ -36,6 +41,7 @@ const CATEGORY_PRESENTATIONS: Readonly> portfolio: {icon: 'collections_bookmark', label: 'Portfolio', tone: 'portfolio'}, extension: {icon: 'more_time', label: 'Extension', tone: 'extension'}, general: {icon: 'campaign', label: 'OnTrack update', tone: 'general'}, + unit_hub: {icon: 'hub', label: 'Unit Hub', tone: 'unit-hub'}, }; const UNKNOWN_PRESENTATION: NotificationPresentation = { diff --git a/src/app/common/notifications/notification-target.spec.ts b/src/app/common/notifications/notification-target.spec.ts index 50c901f50b..0c31fa8f79 100644 --- a/src/app/common/notifications/notification-target.spec.ts +++ b/src/app/common/notifications/notification-target.spec.ts @@ -179,3 +179,78 @@ describe('notificationTarget when there is nothing to open', () => { }); }); }); + +describe('notificationTarget for Unit Hub updates', () => { + function hubEvent(event: string, extra: Partial = {}): Notification { + return notification({ + event, + notificationType: 'unit_hub', + link: '/unit-hub?unit=3&announcement=12', + unitId: UNIT, + projectId: null, + studentId: null, + taskDefinitionAbbr: null, + announcementId: null, + sessionId: null, + ...extra, + }); + } + + it.each(['unit_announcement_published', 'unit_announcement_updated'])( + '%s opens the hub on that announcement for students and staff', + (event) => { + const expected: NotificationTarget = { + kind: 'route', + audience: 'member', + commands: ['/unit-hub'], + queryParams: {unit: UNIT, announcement: 12}, + }; + + expect(notificationTarget(hubEvent(event, {announcementId: 12}), STUDENT)).toEqual(expected); + expect(notificationTarget(hubEvent(event, {announcementId: 12}), TUTOR)).toEqual(expected); + }, + ); + + it.each(['unit_session_changed', 'unit_session_starting_soon'])( + '%s opens the hub on that session', + (event) => { + expect(notificationTarget(hubEvent(event, {sessionId: 40}), STUDENT)).toEqual({ + kind: 'route', + audience: 'member', + commands: ['/unit-hub'], + queryParams: {unit: UNIT, session: 40}, + }); + }, + ); + + it('reports a deleted announcement or session as unavailable', () => { + expect( + notificationTarget(hubEvent('unit_announcement_published', {unitId: null}), STUDENT), + ).toEqual({kind: 'unavailable'}); + expect(notificationTarget(hubEvent('unit_session_changed', {unitId: null}), STUDENT)).toEqual({ + kind: 'unavailable', + }); + }); + + it('opens the hub on the unit for a Unit Hub event it does not know yet', () => { + expect(notificationTarget(hubEvent('unit_hub_future_event'), STUDENT)).toEqual({ + kind: 'route', + audience: 'member', + commands: ['/unit-hub'], + queryParams: {unit: UNIT}, + }); + }); + + it('follows the link from an api that sends no Unit Hub ids', () => { + const legacy = notification({ + event: 'unit_announcement_published', + notificationType: 'unit_hub', + link: '/unit-hub?unit=3&announcement=12', + }); + + expect(notificationTarget(legacy, STUDENT)).toEqual({ + kind: 'link', + link: '/unit-hub?unit=3&announcement=12', + }); + }); +}); diff --git a/src/app/common/notifications/notification-target.ts b/src/app/common/notifications/notification-target.ts index ebc08c499b..2909ea880d 100644 --- a/src/app/common/notifications/notification-target.ts +++ b/src/app/common/notifications/notification-target.ts @@ -6,6 +6,8 @@ import {Notification} from 'src/app/api/models/notification'; * * - `route`: go to `commands` with `queryParams`. `audience` says whose page it * is, so the caller can check a staff member still teaches that unit first. + * `member` is a page for anyone in the unit, such as the Unit Hub, which + * checks access itself. * - `link`: the api is too old to send ids, so follow its link as before. * - `unavailable`: the thing it was about is gone. Say so and stay put. * - `none`: nothing to open, for example a general announcement. @@ -13,7 +15,7 @@ import {Notification} from 'src/app/api/models/notification'; export type NotificationTarget = | { kind: 'route'; - audience: 'student' | 'staff'; + audience: 'student' | 'staff' | 'member'; commands: (string | number)[]; queryParams?: Params; } @@ -37,6 +39,16 @@ const PORTFOLIO_EVENTS: ReadonlySet = new Set([ 'portfolio_submitted', ]); +const UNIT_HUB_ANNOUNCEMENT_EVENTS: ReadonlySet = new Set([ + 'unit_announcement_published', + 'unit_announcement_updated', +]); + +const UNIT_HUB_SESSION_EVENTS: ReadonlySet = new Set([ + 'unit_session_changed', + 'unit_session_starting_soon', +]); + /** * One place that decides where every notification goes, for either role. * @@ -49,6 +61,10 @@ export function notificationTarget( notification: Notification, viewer: NotificationViewer, ): NotificationTarget { + if (notification.notificationType === 'unit_hub') { + return unitHubTarget(notification); + } + const hasIds = notification.projectId !== undefined; if (!hasIds) { @@ -89,6 +105,35 @@ export function notificationTarget( ); } +/** + * A Unit Hub announcement or session opens the hub on its unit, naming the one + * it is about in the query so the hub can open its details. + */ +function unitHubTarget(notification: Notification): NotificationTarget { + const event = notification.event; + const isSession = UNIT_HUB_SESSION_EVENTS.has(event); + const isAnnouncement = UNIT_HUB_ANNOUNCEMENT_EVENTS.has(event); + + // An api that sends no ids at all sends neither of these. + if (notification.announcementId === undefined && notification.sessionId === undefined) { + return notification.link ? {kind: 'link', link: notification.link} : {kind: 'none'}; + } + + const id = isSession ? notification.sessionId : notification.announcementId; + if (notification.unitId == null || ((isSession || isAnnouncement) && id == null)) { + return {kind: 'unavailable'}; + } + + const queryParams: Params = {unit: notification.unitId}; + if (isSession) { + queryParams.session = id; + } else if (isAnnouncement) { + queryParams.announcement = id; + } + + return {kind: 'route', audience: 'member', commands: ['/unit-hub'], queryParams}; +} + function studentTarget( event: string, type: string, diff --git a/src/app/common/panel-layout/panel-fullscreen-button.component.html b/src/app/common/panel-layout/panel-fullscreen-button.component.html new file mode 100644 index 0000000000..14657560ff --- /dev/null +++ b/src/app/common/panel-layout/panel-fullscreen-button.component.html @@ -0,0 +1,14 @@ +@if (visible) { + +} diff --git a/src/app/common/panel-layout/panel-fullscreen-button.component.scss b/src/app/common/panel-layout/panel-fullscreen-button.component.scss new file mode 100644 index 0000000000..39e07a2cbc --- /dev/null +++ b/src/app/common/panel-layout/panel-fullscreen-button.component.scss @@ -0,0 +1,4 @@ +// The button sits in its neighbours' row as if it were written there directly. +:host { + display: contents; +} diff --git a/src/app/common/panel-layout/panel-fullscreen-button.component.ts b/src/app/common/panel-layout/panel-fullscreen-button.component.ts new file mode 100644 index 0000000000..3c4697f48c --- /dev/null +++ b/src/app/common/panel-layout/panel-fullscreen-button.component.ts @@ -0,0 +1,86 @@ +import { + ChangeDetectionStrategy, + Component, + DoCheck, + ElementRef, + Input, + ViewChild, + inject, +} from '@angular/core'; +import {MatButtonModule} from '@angular/material/button'; +import {MatIconModule} from '@angular/material/icon'; +import {MatTooltipModule} from '@angular/material/tooltip'; +import {PanelComponent} from './panel.component'; + +/** + * Takes the `app-panel` it sits in full screen, from a control row inside that panel's own + * content, such as a tab bar. Outside a panel, or while the panels are stacked one at a + * time on a small screen, it renders nothing. + * + * Leaving full screen by any route (this button, Esc) puts focus back on the button, as + * long as focus was still somewhere in the panel. A swap to another full-screen panel + * leaves focus where the user put it. + */ +@Component({ + selector: 'app-panel-fullscreen-button', + changeDetection: ChangeDetectionStrategy.Eager, + imports: [MatButtonModule, MatIconModule, MatTooltipModule], + host: {class: 'app-panel-fullscreen-button'}, + styleUrl: './panel-fullscreen-button.component.scss', + templateUrl: './panel-fullscreen-button.component.html', +}) +export class PanelFullscreenButtonComponent implements DoCheck { + /** Classes for the button, to match the icon buttons beside it. */ + @Input() public buttonClass = ''; + + public readonly panel = inject(PanelComponent, {optional: true}); + + @ViewChild('toggle', {read: ElementRef}) private toggle?: ElementRef; + + private readonly host = inject>(ElementRef); + private wasFullscreen = false; + + public get visible(): boolean { + return !!this.panel && !this.panel.stacked; + } + + public get isFullscreen(): boolean { + return !!this.panel?.isFullscreen; + } + + public get label(): string { + return this.isFullscreen ? 'Exit full screen' : `Open ${this.panel?.panelTitle} full screen`; + } + + public ngDoCheck(): void { + const fullscreen = this.isFullscreen; + if (this.wasFullscreen && !fullscreen) { + // After this check has finished, so a blur elsewhere does not land mid-render. + void Promise.resolve().then(() => this.restoreFocus()); + } + this.wasFullscreen = fullscreen; + } + + public toggleFullscreen(): void { + this.panel?.toggleFullscreen(); + } + + private restoreFocus(): void { + const button = this.toggle?.nativeElement; + // Gone when the layout left full screen because the panels stacked. + if (!button || !this.host.nativeElement.contains(button)) { + return; + } + const panelElement = this.host.nativeElement.closest('app-panel'); + // Another panel went full screen in this one's place, so this one is out of reach. + if (panelElement?.hasAttribute('inert')) { + return; + } + const active = document.activeElement; + const focusWasInPanel = + !active || active === document.body || (!!panelElement && panelElement.contains(active)); + if (focusWasInPanel && active !== button) { + button.focus(); + } + } +} diff --git a/src/app/common/panel-layout/panel-layout.component.spec.ts b/src/app/common/panel-layout/panel-layout.component.spec.ts index 4e137b9879..cd21b3995f 100644 --- a/src/app/common/panel-layout/panel-layout.component.spec.ts +++ b/src/app/common/panel-layout/panel-layout.component.spec.ts @@ -4,6 +4,7 @@ import {ChangeDetectionStrategy, Component} from '@angular/core'; import {ComponentFixture, TestBed} from '@angular/core/testing'; import {BehaviorSubject} from 'rxjs'; import {PanelCollapseButtonComponent} from './panel-collapse-button.component'; +import {PanelFullscreenButtonComponent} from './panel-fullscreen-button.component'; import {PanelLayoutComponent} from './panel-layout.component'; import {PanelStateService} from './panel-state.service'; import {PanelComponent} from './panel.component'; @@ -11,7 +12,12 @@ import {PanelComponent} from './panel.component'; // The real template is set in the test module below, so this one stays empty. @Component({ changeDetection: ChangeDetectionStrategy.Eager, - imports: [PanelLayoutComponent, PanelComponent, PanelCollapseButtonComponent], + imports: [ + PanelLayoutComponent, + PanelComponent, + PanelCollapseButtonComponent, + PanelFullscreenButtonComponent, + ], // eslint-disable-next-line @angular-eslint/component-max-inline-declarations template: '', }) @@ -19,6 +25,28 @@ class HostComponent { public fullscreen: string | null = null; } +// A page that keeps the width itself, the way the project dashboard does. The restored +// width has to land here without moving the parent after its own check has run. +@Component({ + changeDetection: ChangeDetectionStrategy.Eager, + imports: [PanelLayoutComponent, PanelComponent], + // eslint-disable-next-line @angular-eslint/component-max-inline-declarations + template: ` + + + + `, +}) +class TwoWayWidthHostComponent { + public width: number | string | null = 300; +} + const hostTemplate = ` - +
+ + +
+

work

 comments

 +
`; describe('PanelLayoutComponent', () => { @@ -82,7 +115,7 @@ describe('PanelLayoutComponent', () => { window.localStorage.clear(); stacked$ = new BehaviorSubject({matches: false, breakpoints: {}}); await TestBed.configureTestingModule({ - imports: [HostComponent], + imports: [HostComponent, TwoWayWidthHostComponent], providers: [{provide: BreakpointObserver, useValue: {observe: () => stacked$}}], }) .overrideComponent(HostComponent, {set: {template: hostTemplate}}) @@ -160,6 +193,35 @@ describe('PanelLayoutComponent', () => { expect(panel('list').style.minWidth).toBe('280px'); }); + it('does not strand a drag when a second pointer grabs the same handle', () => { + // The teardown for a drag lives in one field. A second pointerdown used to + // overwrite it, leaving the first drag's document listeners on the page for + // good, still resizing the panel from a pointer with no button held. Touch + // reports button 0 for every finger, so this is a two-finger grab. + create(); + const list = instance('list'); + const added: string[] = []; + const removed: string[] = []; + vi.spyOn(document, 'addEventListener').mockImplementation(((type: string) => { + added.push(type); + }) as never); + vi.spyOn(document, 'removeEventListener').mockImplementation(((type: string) => { + removed.push(type); + }) as never); + + const grab = () => new PointerEvent('pointerdown', {button: 0, clientX: 500}); + list.startResize(grab()); + list.startResize(grab()); + + // Whatever the second grab added, the first grab's listeners are gone too. + expect(added.filter((type) => type === 'pointermove').length).toBe(2); + expect(removed.filter((type) => type === 'pointermove').length).toBe(1); + expect(list.resizing).toBe(true); + + vi.mocked(document.addEventListener).mockRestore(); + vi.mocked(document.removeEventListener).mockRestore(); + }); + it('clamps a remembered width that is out of range when it loads', () => { window.localStorage.setItem('ontrack.panels.spec.list', '{"width":90}'); window.localStorage.setItem('ontrack.panels.spec.comments', '{"width":9000}'); @@ -168,6 +230,33 @@ describe('PanelLayoutComponent', () => { expect(instance('comments').width).toBe(640); }); + it('hands a restored width to a two-way parent after its check, never inside it', async () => { + window.localStorage.setItem('ontrack.panels.spec.list', '{"width":420}'); + const host = TestBed.createComponent(TwoWayWidthHostComponent); + + // Emitting straight from ngOnInit moved the parent's `width` after Angular had + // already read it for the `[width]` binding, which is NG0100 in dev mode. + expect(() => host.detectChanges()).not.toThrow(); + expect(host.componentInstance.width).toBe(300); + + await host.whenStable(); + host.detectChanges(); + + expect(host.componentInstance.width).toBe(420); + expect(host.nativeElement.querySelector('app-panel').style.width).toBe('420px'); + }); + + it('drops a restored width when the panel goes before the check it waits for', async () => { + window.localStorage.setItem('ontrack.panels.spec.list', '{"width":420}'); + const host = TestBed.createComponent(TwoWayWidthHostComponent); + host.detectChanges(); + host.destroy(); + + await host.whenStable(); + + expect(host.componentInstance.width).toBe(300); + }); + it('rails the list, then comments, when the page is too narrow, without touching storage', async () => { create(); await Promise.resolve(); @@ -269,6 +358,97 @@ describe('PanelLayoutComponent', () => { expect(panel('list').hasAttribute('inert')).toBe(false); }); + describe('full-screen button', () => { + const fullscreenButton = (): HTMLButtonElement => + panel('work').querySelector('.work-tabs app-panel-fullscreen-button button'); + + it('toggles the panel it sits in, with a label, pressed state and icon to match', () => { + create(); + const toggle = fullscreenButton(); + const spy = vi.spyOn(instance('work'), 'toggleFullscreen'); + + expect(toggle.getAttribute('aria-label')).toBe('Open Selected task full screen'); + expect(toggle.getAttribute('aria-pressed')).toBe('false'); + expect(toggle.classList).toContain('mat-mdc-icon-button'); + expect(toggle.classList).toContain('work-fullscreen'); + expect(toggle.textContent.trim()).toBe('open_in_full'); + + toggle.click(); + fixture.detectChanges(); + + expect(spy).toHaveBeenCalledTimes(1); + expect(fixture.componentInstance.fullscreen).toBe('work'); + expect(panel('work').classList).toContain('app-panel--fullscreen'); + expect(panel('list').hasAttribute('inert')).toBe(true); + expect(toggle.getAttribute('aria-label')).toBe('Exit full screen'); + expect(toggle.getAttribute('aria-pressed')).toBe('true'); + expect(toggle.textContent.trim()).toBe('close_fullscreen'); + + toggle.click(); + fixture.detectChanges(); + + expect(fixture.componentInstance.fullscreen).toBeNull(); + expect(panel('work').classList).not.toContain('app-panel--fullscreen'); + }); + + it('renders nothing outside a panel', () => { + create(); + const outside = fixture.nativeElement.querySelector('.outside-panel'); + expect(outside.querySelector('app-panel-fullscreen-button')).not.toBeNull(); + expect(outside.querySelector('button')).toBeNull(); + }); + + it('renders nothing while the panels are stacked', () => { + create(); + stacked$.next({matches: true, breakpoints: {}}); + fixture.detectChanges(); + expect(fullscreenButton()).toBeNull(); + }); + + it('leaves full screen on Esc and puts focus back on the button', async () => { + create(); + fullscreenButton().click(); + fixture.detectChanges(); + panel('work').querySelector('.work-action').focus(); + expect(document.activeElement).toBe(panel('work').querySelector('.work-action')); + + document.dispatchEvent(new KeyboardEvent('keydown', {key: 'Escape'})); + fixture.detectChanges(); + // Focus moves once the check that saw full screen end has finished. + await Promise.resolve(); + + expect(fixture.componentInstance.fullscreen).toBeNull(); + expect(panel('work').classList).not.toContain('app-panel--fullscreen'); + expect(document.activeElement).toBe(fullscreenButton()); + }); + + it('keeps one panel full screen at a time, swapping from comments to the task', async () => { + create(); + const comments = button('comments', 'Full screen Comments'); + comments.click(); + fixture.detectChanges(); + expect(fixture.componentInstance.fullscreen).toBe('comments'); + + fullscreenButton().click(); + fixture.detectChanges(); + await fixture.whenStable(); + + expect(fixture.componentInstance.fullscreen).toBe('work'); + expect(fixture.nativeElement.querySelectorAll('.app-panel--fullscreen').length).toBe(1); + expect(panel('comments').classList).not.toContain('app-panel--fullscreen'); + expect(panel('comments').hasAttribute('inert')).toBe(true); + expect(panel('work').hasAttribute('inert')).toBe(false); + + button('comments', 'Full screen Comments').click(); + fixture.detectChanges(); + await fixture.whenStable(); + + expect(fixture.componentInstance.fullscreen).toBe('comments'); + expect(fixture.nativeElement.querySelectorAll('.app-panel--fullscreen').length).toBe(1); + expect(fullscreenButton().getAttribute('aria-pressed')).toBe('false'); + }); + }); + it('shows one panel at a time behind tabs when stacked, and ignores collapse', () => { window.localStorage.setItem('ontrack.panels.spec.comments', '{"collapsed":true}'); create(); diff --git a/src/app/common/panel-layout/panel.component.scss b/src/app/common/panel-layout/panel.component.scss index 5d0e575c7a..57c8d76d1e 100644 --- a/src/app/common/panel-layout/panel.component.scss +++ b/src/app/common/panel-layout/panel.component.scss @@ -33,6 +33,25 @@ inset: var(--app-panel-gap, 0.75rem); z-index: 270; width: auto; + animation: app-panel-fullscreen-in 200ms cubic-bezier(0.23, 1, 0.32, 1); +} + +// Opening settles in from just under full size. Leaving is instant, back to the columns. +@keyframes app-panel-fullscreen-in { + from { + opacity: 0; + transform: scale(0.98); + } + to { + opacity: 1; + transform: none; + } +} + +@media (prefers-reduced-motion: reduce) { + :host(.app-panel--fullscreen) { + animation: none; + } } .app-panel__card { diff --git a/src/app/common/panel-layout/panel.component.ts b/src/app/common/panel-layout/panel.component.ts index 28d1caaad9..9bf910bc1f 100644 --- a/src/app/common/panel-layout/panel.component.ts +++ b/src/app/common/panel-layout/panel.component.ts @@ -20,8 +20,9 @@ import {PanelStateService} from './panel-state.service'; * One rounded card in an `app-panel-layout`. Project actions into `[panelActions]` and a * note beside the title into `[panelSubtitle]`; everything else becomes the body. * - * A panel without a header puts an `app-panel-collapse-button` in its content's own - * control row, which finds this panel and collapses it. + * A panel without a header puts an `app-panel-collapse-button` or an + * `app-panel-fullscreen-button` in its content's own control row, which finds this panel + * and collapses it or takes it full screen. */ @Component({ selector: 'app-panel', @@ -68,6 +69,7 @@ export class PanelComponent implements PanelRegistration, OnInit, OnDestroy { private readonly state = inject(PanelStateService); private readonly host = inject>(ElementRef); private removeDragListeners: (() => void) | null = null; + private destroyed = false; @HostBinding('class.app-panel') public readonly baseClass = true; @HostBinding('attr.role') public readonly role = 'region'; @@ -175,15 +177,28 @@ export class PanelComponent implements PanelRegistration, OnInit, OnDestroy { } if (this.resizeEdge && !this.flex && saved.width) { this.width = this.clamp(saved.width); - this.widthChange.emit(this.width); + // A panel runs its ngOnInit inside the check that set its inputs, so emitting here + // changes parent state Angular has already read, which is NG0100 under `[(width)]` + // or any listener that feeds a binding. Hand the width over just after the check + // instead, the way the layout settles its rails. + void Promise.resolve().then(() => this.emitRestoredWidth()); } } public ngOnDestroy(): void { + this.destroyed = true; this.layout?.unregister(this); this.stopDragging(); } + /** The parent still has to hear it: the layout's space arithmetic uses the width it holds. */ + private emitRestoredWidth(): void { + if (this.destroyed || typeof this.width !== 'number') { + return; + } + this.widthChange.emit(this.width); + } + public setCollapsed(collapsed: boolean): void { if (!this.collapsible || this.collapsed === collapsed) { return; @@ -218,6 +233,13 @@ export class PanelComponent implements PanelRegistration, OnInit, OnDestroy { if (!this.canResize || event.button !== 0) { return; } + // Only one drag owns the panel. The teardown for a drag lives in a single + // field, so a second pointerdown before the first pointerup used to + // overwrite it and strand three document listeners: the abandoned move + // handler kept its own startX and startWidth and went on resizing the panel + // from a pointer with no button held, for the rest of the session. Touch + // reports button 0 for every finger, so the guard above does not cover it. + this.stopDragging(); event.preventDefault(); const startX = event.clientX; const startWidth = this.currentWidth; diff --git a/src/app/common/services/confetti.service.ts b/src/app/common/services/confetti.service.ts index 92a51c8ad2..52b38b2b77 100644 --- a/src/app/common/services/confetti.service.ts +++ b/src/app/common/services/confetti.service.ts @@ -5,12 +5,24 @@ import {Injectable} from '@angular/core'; providedIn: 'root', }) export class ConfettiService { - public canon(x: number = 0, y: number = 0, angle = 210): void { + /** + * `options.zIndex` matters over a dialog: the canvas is appended to the body at + * z-index 100 by default, which is under the CDK overlay, so confetti fired + * from inside a dialog lands behind it. + */ + public canon( + x: number = 0, + y: number = 0, + angle = 210, + options: {zIndex?: number; particleCount?: number; spread?: number; scalar?: number} = {}, + ): void { confetti({ angle: angle, - spread: 80, - particleCount: 100, + spread: options.spread ?? 80, + particleCount: options.particleCount ?? 100, origin: {y: y, x: x}, + ...(options.zIndex === undefined ? {} : {zIndex: options.zIndex}), + ...(options.scalar === undefined ? {} : {scalar: options.scalar}), }); } } diff --git a/src/app/common/user-icon/user-icon.component.secure-context.spec.ts b/src/app/common/user-icon/user-icon.component.secure-context.spec.ts new file mode 100644 index 0000000000..65c83f3c0f --- /dev/null +++ b/src/app/common/user-icon/user-icon.component.secure-context.spec.ts @@ -0,0 +1,18 @@ +import {afterEach, describe, expect, it, vi} from 'vitest'; +import {UserIconComponent} from './user-icon.component'; + +describe('UserIconComponent outside a secure context', () => { + afterEach(() => vi.unstubAllGlobals()); + + it('still resolves the background without SubtleCrypto', async () => { + vi.stubGlobal('crypto', {}); + const component = new UserIconComponent({currentUser: null} as never); + component.user = {email: 'demo@example.com', name: 'Demo Student'} as never; + + const url = await ( + component as unknown as {backgroundUrl: () => Promise} + ).backgroundUrl(); + + expect(url).toBeNull(); + }); +}); diff --git a/src/app/common/user-icon/user-icon.component.ts b/src/app/common/user-icon/user-icon.component.ts index 212eee6e02..fbe3ac907b 100644 --- a/src/app/common/user-icon/user-icon.component.ts +++ b/src/app/common/user-icon/user-icon.component.ts @@ -72,13 +72,26 @@ export class UserIconComponent implements AfterViewInit, OnChanges { constructor(private userService: UserService) {} - private async backgroundUrl(): Promise { - const hash = await this.sha256(this.email?.trim().toLowerCase() ?? ''); - return `https://www.gravatar.com/avatar/${hash}.png?default=blank&size=${this.size * 4}`; + /** + * The Gravatar photo behind the initials, or null when it cannot be worked out. + * crypto.subtle only exists on https or localhost, so opening OnTrack by a LAN + * address threw here and the avatar never drew at all. Without a hash the + * initials still show. + */ + private async backgroundUrl(): Promise { + try { + const hash = await this.sha256(this.email?.trim().toLowerCase() ?? ''); + return `https://www.gravatar.com/avatar/${hash}.png?default=blank&size=${this.size * 4}`; + } catch { + return null; + } } private async sha256(value: string): Promise { const bytes = new TextEncoder().encode(value); + if (!globalThis.crypto?.subtle) { + throw new Error('SubtleCrypto is unavailable outside a secure context'); + } const digest = await crypto.subtle.digest('SHA-256', bytes); return Array.from(new Uint8Array(digest), (byte) => byte.toString(16).padStart(2, '0')).join( '', @@ -239,13 +252,15 @@ export class UserIconComponent implements AfterViewInit, OnChanges { .attr('fill', 'white') .text((d) => d.text); - svg - .append('image') - .attr('xlink:href', backgroundUrl) - .attr('width', this.size) - .attr('height', this.size) - .attr('x', 0) - .attr('y', 0) - .attr('clip-path', `url(#image-clip-${id})`); + if (backgroundUrl) { + svg + .append('image') + .attr('xlink:href', backgroundUrl) + .attr('width', this.size) + .attr('height', this.size) + .attr('x', 0) + .attr('y', 0) + .attr('clip-path', `url(#image-clip-${id})`); + } } } diff --git a/src/app/dashboard/f-cross-dashboard.component.html b/src/app/dashboard/f-cross-dashboard.component.html index cb2eabe776..b470f1303f 100644 --- a/src/app/dashboard/f-cross-dashboard.component.html +++ b/src/app/dashboard/f-cross-dashboard.component.html @@ -365,11 +365,18 @@

@for (mode of filterOptions; track mode) { +
- + {{ mode }}
} diff --git a/src/app/dashboard/f-cross-dashboard.component.spec.ts b/src/app/dashboard/f-cross-dashboard.component.spec.ts index 56924ffdb7..2fb83e5d4b 100644 --- a/src/app/dashboard/f-cross-dashboard.component.spec.ts +++ b/src/app/dashboard/f-cross-dashboard.component.spec.ts @@ -7,6 +7,7 @@ import {ReactiveFormsModule} from '@angular/forms'; import {provideDateFnsAdapter} from '@angular/material-date-fns-adapter'; import {MatButtonModule} from '@angular/material/button'; import {MatButtonHarness} from '@angular/material/button/testing'; +import {MatCheckboxModule} from '@angular/material/checkbox'; import {MAT_DATE_LOCALE} from '@angular/material/core'; import {MatDatepickerModule} from '@angular/material/datepicker'; import {MatDateRangeInputHarness} from '@angular/material/datepicker/testing'; @@ -127,6 +128,7 @@ describe('CrossDashboardComponent', () => { declarations: [CrossDashboardComponent], imports: [ MatButtonModule, + MatCheckboxModule, MatDatepickerModule, MatFormFieldModule, MatIconModule, @@ -968,6 +970,31 @@ describe('CrossDashboardComponent', () => { ]); }); + // The text in the menu item is not a label element, so the box needs its own name. + it('names the per-unit filter box after the filter it turns on', async () => { + projectsSubject.next([ + makeProject(1, 'SIT764', true, [ + makeTask('Individual Retrospective', '5.1P', 'not_started', makeDate(12)), + ]), + ]); + + await syncView(); + + const loader = TestbedHarnessEnvironment.loader(fixture); + const filterButton = await loader.getHarness( + MatButtonHarness.with({selector: '[aria-label^="Filter tasks in"]'}), + ); + await filterButton.click(); + await syncView(); + + const item = document.querySelector('.mat-mdc-menu-panel .mat-mdc-menu-item') as HTMLElement; + const box = item?.querySelector('input') as HTMLInputElement; + + expect(item?.textContent.trim()).toBe('Hide Completed'); + expect(box?.getAttribute('aria-label')).toBe('Hide Completed'); + expect(item?.getAttribute('aria-label')).toBe('Hide Completed'); + }); + it('binds both multiple-select controls and Clear all through the rendered toolbar', async () => { projectsSubject.next([ makeProject(1, 'SIT764', true, [ diff --git a/src/app/doubtfire-angular.module.ts b/src/app/doubtfire-angular.module.ts index 08ccf4bc90..e1e1a86bbb 100644 --- a/src/app/doubtfire-angular.module.ts +++ b/src/app/doubtfire-angular.module.ts @@ -25,6 +25,7 @@ import {environment} from 'src/environments/environment'; import {ClipboardModule} from '@angular/cdk/clipboard'; import {DragDropModule} from '@angular/cdk/drag-drop'; import {ScrollingModule} from '@angular/cdk/scrolling'; +import {TextFieldModule} from '@angular/cdk/text-field'; import {HTTP_INTERCEPTORS, HttpClientModule} from '@angular/common/http'; import {APP_INITIALIZER, ErrorHandler, Injector, NgModule} from '@angular/core'; import {FormsModule, ReactiveFormsModule} from '@angular/forms'; @@ -164,6 +165,8 @@ import {ArchiveViewerComponent} from './common/archive-viewer/archive-viewer.com import {AudioPlayerComponent} from './common/audio-player/audio-player.component'; import {AudioCommentRecorderComponent} from './common/audio-recorder/audio/audio-comment-recorder/audio-comment-recorder'; import {MicrophoneTesterComponent} from './common/audio-recorder/audio/microphone-tester/microphone-tester.component'; +import {AnimatedCheckComponent} from './common/celebrate/animated-check.component'; +import {CelebrationParticlesComponent} from './common/celebrate/celebration-particles.component'; import {ChartBaseComponent} from './common/chart-base/chart-base-component/chart-base-component.component'; import {DragDropDirective} from './common/directives/drag-drop.directive'; import {EditProfileFormComponent} from './common/edit-profile-form/edit-profile-form.component'; @@ -215,6 +218,7 @@ import {NotificationsPageComponent} from './common/notifications-page/notificati import {ObjectSelectComponent} from './common/obect-select/object-select.component'; import {PageContainerComponent} from './common/page-container/page-container.component'; import {PanelCollapseButtonComponent} from './common/panel-layout/panel-collapse-button.component'; +import {PanelFullscreenButtonComponent} from './common/panel-layout/panel-fullscreen-button.component'; import {PanelLayoutComponent} from './common/panel-layout/panel-layout.component'; import {PanelComponent} from './common/panel-layout/panel.component'; import {PdfViewerPanelComponent} from './common/pdf-viewer-panel/pdf-viewer-panel.component'; @@ -821,8 +825,11 @@ const DEFAULT_TOOLTIP_OPTIONS: MatTooltipDefaultOptions = { PanelLayoutComponent, PanelComponent, PanelCollapseButtonComponent, + PanelFullscreenButtonComponent, ThemeToggleComponent, EmptyStateComponent, + AnimatedCheckComponent, + CelebrationParticlesComponent, FlexLayoutModule, BrowserModule, BrowserAnimationsModule, @@ -835,6 +842,11 @@ const DEFAULT_TOOLTIP_OPTIONS: MatTooltipDefaultOptions = { ClipboardModule, DragDropModule, ScrollingModule, + // Seven templates ask a textarea to grow with its content, and set a minimum + // of three rows while they are at it. Without this the directive is an inert + // attribute, so every one of them rendered at the browser default of two rows + // and never grew. + TextFieldModule, MatToolbarModule, MatSidenavModule, MatFormFieldModule, diff --git a/src/app/home/splash-screen/splash-screen.component.html b/src/app/home/splash-screen/splash-screen.component.html index c8357a173a..79ca0e76ff 100644 --- a/src/app/home/splash-screen/splash-screen.component.html +++ b/src/app/home/splash-screen/splash-screen.component.html @@ -1,6 +1,7 @@ @if (startupState$ | async; as state) { @if (state.status === 'loading' && state.phase === 'authentication') {
-
} @else if (state.status === 'loading') { @@ -23,8 +52,10 @@ data-testid="startup-data-loading" role="status" > - - {{ state.message }} + + @if (state.message) { + {{ state.message }} + } } @else if (state.status === 'error' || state.status === 'offline') {
{ ).toBeTruthy(); }); + it('draws the mark, the wordmark and the current status while signing in', () => { + const splash = fixture.nativeElement.querySelector('[data-testid="startup-auth-loading"]'); + const mark = splash.querySelector('[data-testid="startup-mark"]'); + + expect(splash.getAttribute('role')).toBe('status'); + expect(splash.getAttribute('aria-live')).toBe('polite'); + expect(mark.querySelectorAll('svg path')).toHaveLength(3); + expect(mark.getAttribute('aria-hidden')).toBe('true'); + expect(splash.querySelector('[data-testid="startup-wordmark"]').textContent.trim()).toBe( + 'OnTrack', + ); + expect(splash.querySelector('[data-testid="startup-status"]').textContent.trim()).toBe( + 'Checking your session…', + ); + expect(splash.querySelector('img')).toBeFalsy(); + }); + + it('swaps the status text in place when the message changes', () => { + startupState.next({ + status: 'loading', + phase: 'authentication', + message: 'Still checking your session…', + attempt: 2, + startedAt: Date.now(), + }); + fixture.detectChanges(); + + const statuses = fixture.nativeElement.querySelectorAll('[data-testid="startup-status"]'); + expect(statuses).toHaveLength(1); + expect(statuses[0].textContent.trim()).toBe('Still checking your session…'); + }); + + it('shows a thin progress line with the message as a caption while data loads', () => { + startupState.next({ + status: 'loading', + phase: 'units-and-projects', + message: 'Loading your units…', + attempt: 1, + startedAt: Date.now(), + }); + fixture.detectChanges(); + + const progress = fixture.nativeElement.querySelector('[data-testid="startup-data-loading"]'); + expect(progress.getAttribute('role')).toBe('status'); + expect(progress.getAttribute('aria-live')).toBe('polite'); + expect(progress.querySelector('.startup-progress-track span')).toBeTruthy(); + expect(progress.querySelector('.startup-progress-caption').textContent.trim()).toBe( + 'Loading your units…', + ); + }); + it('renders a terminal recovery action instead of an indefinite logo', () => { startupState.next({ status: 'error', diff --git a/src/app/projects/states/dashboard/directives/task-dashboard/directives/task-description-card/ppi-widget/ppi-widget.component.html b/src/app/projects/states/dashboard/directives/task-dashboard/directives/task-description-card/ppi-widget/ppi-widget.component.html index ea67a1f685..c517cfd623 100644 --- a/src/app/projects/states/dashboard/directives/task-dashboard/directives/task-description-card/ppi-widget/ppi-widget.component.html +++ b/src/app/projects/states/dashboard/directives/task-dashboard/directives/task-description-card/ppi-widget/ppi-widget.component.html @@ -17,8 +17,8 @@ - - Peer Progress Indicator + + Peer progress Anonymous peers at your target grade
@@ -34,16 +34,16 @@ [attr.aria-label]="advancedToggleLabel" (click)="setAdvanced(!advanced)" > + Advanced - Advanced } @if (isCompactResult) { -
+
-
- -
- +
+
+ + @if (hasCompletedPercentage && hasSubmittedPercentage) { +

+ + {{ formatPercentage(view.data?.submittedPercentage ?? 0) }}% have submitted +

+ }
@if (advanced) { -
-
- @if (hasCompletedPercentage) { -
- Completed - {{ formatPercentage(view.data?.completedPercentage ?? 0) }}% -
- } - @if (hasSubmittedPercentage) { -
- Submitted - {{ formatPercentage(view.data?.submittedPercentage ?? 0) }}% -
- } -
- +
-
- Task status breakdown - Independently privacy-rounded; values may not total 100% -
+ Where peers are + Independently privacy-rounded; values may not total 100%
@if (!view.data?.distributionAvailable) { @@ -121,31 +111,69 @@

} @else if (usesStackedDistribution) { - +
+
+ -
    - @for (segment of displaySegments; track segment.status) { -
  • + @if (highlightedSegment; as active) { - {{ segment.label }} - {{ formatPercentage(segment.percentage) }}% -
    • - } -
+ class="ppi-tooltip" + [style.--ppi-tooltip-left]="segmentMidpoint(active.status)" + > + {{ active.label }} · {{ formatPercentage(active.percentage) }}% + + } +
+ +
    + @for (segment of displaySegments; track segment.status) { +
  • + + {{ segment.label }} + {{ formatPercentage(segment.percentage) }}% +
    • + } +
+
} @else {

@@ -162,7 +190,7 @@ class="ppi-swatch" [style.background-color]="segment.color" > - {{ segment.label }} + {{ segment.label }}

} - - @if (view.data?.lastUpdatedAt) { - - Updated {{ view.data?.lastUpdatedAt | date: 'd MMM, h:mm a' }} - - } } + + @if (view.data?.lastUpdatedAt) { +
+ + Updated {{ view.data?.lastUpdatedAt | date: 'd MMM, h:mm a' }} + +
+ } } @else {
@switch (view.state) { diff --git a/src/app/projects/states/dashboard/directives/task-dashboard/directives/task-description-card/ppi-widget/ppi-widget.component.scss b/src/app/projects/states/dashboard/directives/task-dashboard/directives/task-description-card/ppi-widget/ppi-widget.component.scss index 60207a2e67..58b042d3ce 100644 --- a/src/app/projects/states/dashboard/directives/task-dashboard/directives/task-description-card/ppi-widget/ppi-widget.component.scss +++ b/src/app/projects/states/dashboard/directives/task-dashboard/directives/task-description-card/ppi-widget/ppi-widget.component.scss @@ -1,63 +1,67 @@ :host { container-type: inline-size; display: block; + max-width: 100%; min-width: 0; } +$ppi-ease-out: cubic-bezier(0.23, 1, 0.32, 1); + // A flat card like the other task cards: surface, hairline border, the shared -// radius and elevation. The corner glow and the coloured state borders are gone; -// the state icon and the badge in the header already say when data is stale or -// failed, so the frame no longer shouts it as well. +// radius and elevation. Sections are separated by spacing, not rules. .ppi-card { background: var(--ot-color-surface-raised); border: 1px solid var(--ot-color-divider); border-radius: var(--ot-radius-md); - box-sizing: border-box; box-shadow: var(--ot-elevation-1); + box-sizing: border-box; color: var(--ot-color-text); + display: grid; + gap: 1rem; + grid-template-columns: minmax(0, 1fr); max-width: 100%; - overflow: hidden; + min-width: 0; + overflow: clip; + padding: 1rem 1.25rem; &.is-error { border-color: var(--ot-color-error); } + &.is-warning { border-color: var(--ot-color-warning); } - &.is-muted { - background: var(--ot-color-surface); - border-color: var(--ot-color-border); - } } +// ---------------------------------------------------------------- Header + .ppi-head { - align-items: center; + align-items: flex-start; display: flex; - gap: 10px; + gap: 12px; justify-content: space-between; - padding: 1rem 1rem 0.5rem; + min-width: 0; } .ppi-identity { align-items: center; display: flex; - gap: 8px; + gap: 10px; min-width: 0; +} - > span:last-child { - min-width: 0; - } +.ppi-heading-text { + min-width: 0; } .ppi-avatar { align-items: center; background: var(--ot-color-selected); - border: 1px solid var(--ot-color-divider); border-radius: 50%; color: var(--ot-color-primary); display: inline-flex; - flex: 0 0 2.5rem; - height: 2.5rem; + flex: 0 0 2.25rem; + height: 2.25rem; justify-content: center; mat-icon { @@ -74,60 +78,73 @@ .ppi-title { font-size: 1rem; - font-weight: 700; + font-weight: 600; letter-spacing: -0.01em; - line-height: 1.25; + line-height: 1.25rem; } .ppi-subtitle { color: var(--ot-color-text-muted); font-size: 0.8125rem; - line-height: 1.3; - margin-top: 1px; + line-height: 1.125rem; } +// A compact labelled switch. Its visual box is one title line tall so it sits on +// the title row, and a pseudo-element keeps a 44px touch target around it. .ppi-toggle { align-items: center; appearance: none; background: transparent; border: 0; border-radius: 999px; - color: var(--ot-color-text); + color: var(--ot-color-text-muted); cursor: pointer; display: inline-flex; flex: 0 0 auto; font: inherit; font-size: 0.8125rem; - font-weight: 600; - gap: 5px; - min-height: 44px; - padding: 0.5rem 0.25rem; + font-weight: 500; + gap: 8px; + height: 1.25rem; + padding: 0; + position: relative; white-space: nowrap; + &::before { + content: ''; + inset: -12px -8px; + position: absolute; + } + &:focus-visible { outline: 2px solid var(--ot-color-focus); - outline-offset: 2px; + outline-offset: 4px; + } + + &[aria-checked='true'] { + color: var(--ot-color-text); } } .ppi-toggle-track { - background: var(--ot-color-border); + background: var(--ot-color-control-border); border-radius: 999px; + box-sizing: border-box; display: inline-flex; - flex: 0 0 2.25rem; - height: 1.25rem; + flex: 0 0 2rem; + height: 1.125rem; padding: 2px; - transition: background 160ms ease; + transition: background-color 160ms ease; } .ppi-toggle-thumb { - background: var(--ot-color-surface); + background: var(--ot-color-surface-raised); border-radius: 50%; - box-shadow: 0 1px 2px rgb(0 0 0 / 28%); - height: 1rem; + box-shadow: var(--ot-elevation-1); + height: 0.875rem; transform: translateX(0); - transition: transform 160ms ease; - width: 1rem; + transition: transform 160ms $ppi-ease-out; + width: 0.875rem; } .ppi-toggle[aria-checked='true'] .ppi-toggle-track { @@ -135,150 +152,161 @@ } .ppi-toggle[aria-checked='true'] .ppi-toggle-thumb { - transform: translateX(1rem); + transform: translateX(0.875rem); } +// ---------------------------------------------------------------- Summary + .ppi-summary { - align-items: center; display: grid; - gap: 12px; - grid-template-columns: minmax(0, 0.9fr) minmax(0, 1.25fr); - padding: 0.25rem 1rem 1rem 4.5rem; + gap: 0.625rem; + min-width: 0; } .ppi-value { align-items: baseline; + column-gap: 0.625rem; display: flex; - gap: 6px; + flex-wrap: wrap; min-width: 0; + row-gap: 2px; strong { - color: var(--ot-color-primary); - font-size: clamp(1.75rem, 7cqi, 2.5rem); - letter-spacing: -0.04em; + color: var(--ot-color-text); + font-size: 2.25rem; + font-variant-numeric: tabular-nums; + font-weight: 600; + letter-spacing: -0.02em; line-height: 1; } span { + color: var(--ot-color-text); font-size: 0.875rem; - font-weight: 600; - line-height: 1.25; + line-height: 1.3; + min-width: 0; } } -.ppi-visual { - min-width: 0; -} - -.ppi-track, -.ppi-independent__track { - border-radius: 999px; - overflow: hidden; -} - .ppi-track { background: var(--ot-meter-track); - height: 9px; + border-radius: 999px; + height: 8px; + overflow: hidden; } -.ppi-fill, -.ppi-independent__fill { +.ppi-fill { + background: var(--ot-status-complete-graphic, var(--ot-color-success)); border-radius: inherit; display: block; height: 100%; + transform-origin: left center; + transition: width 200ms ease; } -.ppi-fill { - background: var(--ot-color-primary); - transition: width 260ms ease; +.ppi-track.is-submitted .ppi-fill { + background: var(--ot-status-ready-for-feedback-graphic, var(--ot-color-info)); } -.ppi-scale { +.ppi-secondary { + align-items: center; color: var(--ot-color-text-muted); display: flex; - font-size: 10px; - font-weight: 600; - justify-content: space-between; - margin-top: 3px; -} - -.ppi-advanced { - background: var(--ot-color-surface-raised); - border-top: 1px solid var(--ot-color-divider); - display: grid; - gap: 9px; - padding: 1rem; -} - -.ppi-metrics { - display: grid; + font-size: 0.8125rem; + font-variant-numeric: tabular-nums; gap: 6px; - grid-template-columns: repeat(2, minmax(0, 1fr)); + line-height: 1.3; + margin: 0; } -.ppi-metric { - align-items: center; - background: var(--ot-color-surface); - border: 1px solid var(--ot-color-divider); - border-left: 4px solid; - border-radius: 6px; - display: flex; - gap: 12px; - justify-content: space-between; - min-width: 0; - padding: 5px 8px; - - span { - color: var(--ot-color-text-muted); - font-size: 12px; - font-weight: 600; - } +.ppi-dot { + border-radius: 50%; + flex: 0 0 8px; + height: 8px; + width: 8px; - strong { - font-size: 16px; + &.is-submitted { + background: var(--ot-status-ready-for-feedback-graphic, var(--ot-color-info)); } +} - &.is-complete { - border-left-color: var(--ot-color-success); - } +// ---------------------------------------------------------------- Breakdown - &.is-submitted { - border-left-color: var(--ot-color-info); - } +.ppi-advanced { + display: grid; + gap: 0.75rem; + min-width: 0; + padding-top: 0.25rem; } .ppi-heading { - small { - display: block; - } + display: grid; + gap: 2px; strong { - font-size: 14px; + font-size: 0.875rem; + font-weight: 600; + line-height: 1.3; } small { color: var(--ot-color-text-muted); - font-size: 11px; - margin-top: 2px; + font-size: 0.75rem; + line-height: 1.35; } } +.ppi-breakdown { + display: grid; + gap: 0.75rem; + min-width: 0; +} + +.ppi-distribution-wrap { + position: relative; +} + +// One flat track. The 2px gaps are transparent, so the card surface shows +// through and neighbouring segments stay separable in both themes. .ppi-distribution { - background: var(--ot-meter-track); - border: 2px solid var(--ot-color-surface); border-radius: 999px; - box-shadow: 0 1px 4px rgb(0 0 0 / 18%); display: flex; - height: 14px; + gap: 2px; + height: 10px; overflow: hidden; } .ppi-segment { flex-basis: 0; + min-width: 2px; + transform-origin: left center; + transition: + flex-grow 200ms ease, + opacity 150ms ease; +} - & + & { - border-left: 1px solid rgb(255 255 255 / 58%); - } +// Hidden until emphasis is styled; see the focus and hover rules below. +.ppi-tooltip { + background: var(--ot-color-inverse-surface); + border-radius: 6px; + bottom: calc(100% + 6px); + box-shadow: var(--ot-elevation-1); + color: var(--ot-color-inverse-text); + display: none; + font-size: 0.75rem; + font-variant-numeric: tabular-nums; + font-weight: 500; + left: clamp(4.5rem, var(--ppi-tooltip-left, 50%), calc(100% - 4.5rem)); + line-height: 1.2; + max-width: 9rem; + overflow: hidden; + padding: 4px 8px; + pointer-events: none; + position: absolute; + text-overflow: ellipsis; + transform: translateX(-50%); + white-space: nowrap; + z-index: 1; } .ppi-legend, @@ -290,65 +318,111 @@ .ppi-legend { display: grid; - gap: 4px 6px; - grid-template-columns: repeat(auto-fit, minmax(180px, 1fr)); + gap: 0 1.5rem; + grid-template-columns: minmax(0, 1fr); li { align-items: center; - background: var(--ot-color-surface); - border: 1px solid var(--ot-color-divider); - border-radius: 5px; + border-radius: 6px; display: grid; - font-size: 10px; - gap: 5px; - grid-template-columns: 9px minmax(0, 1fr) auto; - min-height: 20px; - padding: 2px 4px; + font-size: 0.8125rem; + gap: 8px; + grid-template-columns: 8px minmax(0, 1fr) auto; + line-height: 1.3; + margin-inline: -6px; + min-height: 1.75rem; + min-width: 0; + padding: 2px 6px; + transition: background-color 150ms ease; + + &:focus-visible { + outline: 2px solid var(--ot-color-focus); + outline-offset: -2px; + } + + strong { + font-variant-numeric: tabular-nums; + font-weight: 600; + text-align: right; + } } } .ppi-swatch { - border: 1px solid rgb(0 0 0 / 12%); border-radius: 50%; - height: 9px; - width: 9px; + height: 8px; + width: 8px; } .ppi-name { + min-width: 0; overflow: hidden; text-overflow: ellipsis; white-space: nowrap; } +// Emphasis: keyboard focus always shows it; pointer hover only where hover is real. +.ppi-breakdown.is-focus-highlighting { + .ppi-segment:not(.is-active) { + opacity: 0.4; + } + + .ppi-legend li.is-active { + background: var(--ot-color-hover); + } +} + +.ppi-breakdown.is-focus-highlighting .ppi-tooltip { + display: block; +} + +@media (hover: hover) and (pointer: fine) { + .ppi-breakdown.is-hover-highlighting { + .ppi-segment:not(.is-active) { + opacity: 0.4; + } + + .ppi-legend li.is-active { + background: var(--ot-color-hover); + } + + .ppi-tooltip { + display: block; + } + } +} + +// ---------------------------------------------------------------- Independent scale + .ppi-independent { display: grid; - gap: 7px; + gap: 0.75rem; + min-width: 0; } .ppi-rounding { - background: var(--ot-color-surface); - border-left: 3px solid var(--ot-color-primary); - border-radius: 6px; color: var(--ot-color-text-muted); - font-size: 11px; + font-size: 0.75rem; line-height: 1.45; margin: 0; - padding: 5px 7px; } .ppi-list { display: grid; - gap: 5px; + gap: 0.5rem; li { align-items: center; display: grid; - font-size: 11px; - gap: 7px; - grid-template-columns: minmax(110px, 0.55fr) minmax(80px, 1fr) 34px; + font-size: 0.8125rem; + gap: 10px; + grid-template-columns: minmax(0, 9rem) minmax(0, 1fr) 2.75rem; + min-width: 0; } strong { + font-variant-numeric: tabular-nums; + font-weight: 600; text-align: right; } } @@ -356,51 +430,78 @@ .ppi-status { align-items: center; display: flex; - font-weight: 600; - gap: 7px; + gap: 8px; min-width: 0; -} -.ppi-status .ppi-swatch { - flex: 0 0 9px; + .ppi-swatch { + flex: 0 0 8px; + } } .ppi-independent__track { background: var(--ot-meter-track); + border-radius: 999px; display: block; - height: 9px; + height: 8px; + overflow: hidden; +} + +.ppi-independent__fill { + border-radius: inherit; + display: block; + height: 100%; + transition: width 200ms ease; } .ppi-notice { align-items: flex-start; - background: var(--ot-color-surface-raised); - border: 1px solid var(--ot-color-warning); - border-radius: 8px; - color: var(--ot-color-warning); + background: var(--ot-color-surface); + border-radius: var(--ot-radius-sm); + color: var(--ot-color-text-muted); display: flex; - font-size: 12px; + font-size: 0.8125rem; gap: 10px; line-height: 1.45; - padding: 8px 9px; + padding: 0.625rem 0.75rem; + + > mat-icon { + color: var(--ot-color-text-muted); + flex: 0 0 auto; + font-size: 20px; + height: 20px; + width: 20px; + } strong { + color: var(--ot-color-text); display: block; + font-weight: 600; margin-bottom: 1px; } } +// ---------------------------------------------------------------- Footer + +.ppi-footer { + margin-top: -0.25rem; +} + .ppi-updated { color: var(--ot-color-text-muted); - font-size: 10px; - justify-self: end; + display: block; + font-size: 0.75rem; + line-height: 1.3; } +// ---------------------------------------------------------------- Non-result states + .ppi-state { align-items: center; display: flex; - gap: 12px; - min-height: 42px; - padding: 4px 16px 14px 58px; + flex-wrap: wrap; + gap: 8px 12px; + min-height: 2.5rem; + min-width: 0; > mat-icon { color: var(--ot-color-text-muted); @@ -409,14 +510,16 @@ > span:not(.ppi-badge) { color: var(--ot-color-text-muted); - flex: 1 1 auto; - font-size: 12px; + flex: 1 1 12rem; + font-size: 0.8125rem; line-height: 1.4; + min-width: 0; strong { color: var(--ot-color-text); display: block; - font-size: 13px; + font-size: 0.875rem; + font-weight: 600; margin-bottom: 1px; } } @@ -431,63 +534,57 @@ } .ppi-badge { - background: var(--ot-color-surface-raised); + background: var(--ot-color-surface); border-radius: 999px; color: var(--ot-color-warning); - font-size: 10px; - font-weight: 700; - padding: 4px 8px; + font-size: 0.75rem; + font-weight: 600; + padding: 3px 8px; white-space: nowrap; } -@container (width < 460px) { - .ppi-head { - align-items: flex-start; - } +// ---------------------------------------------------------------- Entrance motion - .ppi-subtitle { - max-width: 25rem; +@keyframes ppi-grow { + from { + transform: scaleX(0); } - .ppi-summary { - padding-left: 16px; + to { + transform: scaleX(1); } +} - .ppi-state { - align-items: flex-start; - flex-wrap: wrap; - padding-left: 16px; - - button { - margin-left: 30px; - } - } +.ppi-summary.is-entering .ppi-fill { + animation: ppi-grow 400ms $ppi-ease-out both; } -@container (width < 350px) { - .ppi-head { - flex-wrap: wrap; - } +.ppi-advanced.is-entering .ppi-segment { + animation: ppi-grow 400ms $ppi-ease-out both; + animation-delay: calc(var(--ppi-index, 0) * 30ms); +} - .ppi-toggle { - margin-left: 42px; - } +// ---------------------------------------------------------------- Widths - .ppi-value strong { - font-size: 25px; +@container (width >= 420px) { + .ppi-legend { + grid-auto-flow: column; + grid-template-columns: repeat(2, minmax(0, 1fr)); + grid-template-rows: repeat(var(--ppi-legend-rows, 1), auto); } +} - .ppi-summary { - grid-template-columns: 1fr; +@container (width < 380px) { + .ppi-card { + padding: 1rem; } - .ppi-legend { - grid-template-columns: 1fr; + .ppi-value strong { + font-size: 2rem; } .ppi-list li { - align-items: start; - grid-template-columns: 1fr auto; + grid-template-columns: minmax(0, 1fr) auto; } .ppi-independent__track { @@ -496,10 +593,24 @@ } } +@container (width < 300px) { + .ppi-head { + flex-wrap: wrap; + } +} + @media (prefers-reduced-motion: reduce) { .ppi-fill, + .ppi-segment, + .ppi-independent__fill, + .ppi-legend li, .ppi-toggle-track, .ppi-toggle-thumb { transition: none; } + + .ppi-summary.is-entering .ppi-fill, + .ppi-advanced.is-entering .ppi-segment { + animation: none; + } } diff --git a/src/app/projects/states/dashboard/directives/task-dashboard/directives/task-description-card/ppi-widget/ppi-widget.component.spec.ts b/src/app/projects/states/dashboard/directives/task-dashboard/directives/task-description-card/ppi-widget/ppi-widget.component.spec.ts index 27cfc5ba2a..702b1f2ea5 100644 --- a/src/app/projects/states/dashboard/directives/task-dashboard/directives/task-description-card/ppi-widget/ppi-widget.component.spec.ts +++ b/src/app/projects/states/dashboard/directives/task-dashboard/directives/task-description-card/ppi-widget/ppi-widget.component.spec.ts @@ -101,6 +101,30 @@ describe('PpiWidgetComponent', () => { expect(fixture.nativeElement.querySelector('.ppi-fill').style.width).toBe('0%'); }); + it('shows the headline percentage once, with submitted as a single secondary line', () => { + load(of(NORMAL_STATE)); + + const card = fixture.nativeElement.querySelector('.ppi-card') as HTMLElement; + expect(card.textContent.match(/(^|\D)10%/g)?.length).toBe(1); + expect(card.querySelector('.ppi-value strong')?.textContent).toBe('10%'); + expect(card.querySelector('.ppi-secondary')?.textContent.trim()).toBe('60% have submitted'); + expect(card.querySelectorAll('.ppi-track').length).toBe(1); + expect(card.querySelector('.ppi-metric')).toBeNull(); + expect(card.querySelector('.ppi-scale')).toBeNull(); + }); + + it('renders the suppressed state as a calm message with no bars', () => { + load(of(SUPPRESSED_STATE)); + + const card = fixture.nativeElement.querySelector('.ppi-card') as HTMLElement; + expect(card.querySelector('.ppi-state mat-icon')?.textContent).toContain('privacy_tip'); + expect(card.textContent).toContain('Progress is hidden to protect privacy'); + expect(card.querySelector('.ppi-track')).toBeNull(); + expect(card.querySelector('.ppi-distribution')).toBeNull(); + expect(card.querySelector('[role="progressbar"]')).toBeNull(); + expect(card.querySelector('button[role="switch"]')).toBeNull(); + }); + it('shows the API-provided hidden message for a suppressed response', () => { load(of(SUPPRESSED_STATE)); expect(component.view.state).toBe('hidden'); @@ -282,7 +306,7 @@ describe('PpiWidgetComponent', () => { fixture.detectChanges(); const text = fixture.nativeElement.textContent; - expect(text).toContain('Task status breakdown'); + expect(text).toContain('Where peers are'); expect(text).toContain('Redo'); expect(text).toContain('Resubmit'); expect(fixture.nativeElement.querySelector('.ppi-distribution')).toBeTruthy(); @@ -303,6 +327,86 @@ describe('PpiWidgetComponent', () => { ]); }); + it('lists every non-zero status with its percentage in the legend and the bar label', () => { + load(of(NORMAL_STATE)); + component.setAdvanced(true); + fixture.detectChanges(); + + const rows = Array.from( + fixture.nativeElement.querySelectorAll('.ppi-legend li'), + ).map((row) => [ + row.querySelector('.ppi-name')?.textContent.trim(), + row.querySelector('strong')?.textContent.trim(), + ]); + expect(rows).toEqual([ + ['Not Started', '20%'], + ['Working On It', '20%'], + ['Ready for Feedback', '20%'], + ['Resubmit', '10%'], + ['Redo', '10%'], + ['Complete', '10%'], + ['Fail', '10%'], + ]); + + const bar = fixture.nativeElement.querySelector('.ppi-distribution') as HTMLElement; + expect(bar.getAttribute('role')).toBe('img'); + const label = bar.getAttribute('aria-label') ?? ''; + rows.forEach(([name, percentage]) => { + expect(label).toContain(`${name} ${percentage}`); + }); + expect(label).not.toContain('Discuss'); + expect(fixture.nativeElement.querySelectorAll('.ppi-segment').length).toBe(7); + }); + + it('highlights the matching segment while a legend row is hovered or focused', () => { + load(of(NORMAL_STATE)); + component.setAdvanced(true); + fixture.detectChanges(); + + const breakdown = fixture.nativeElement.querySelector('.ppi-breakdown') as HTMLElement; + const row = fixture.nativeElement.querySelector( + '.ppi-legend li[data-status="redo"]', + ) as HTMLElement; + + row.dispatchEvent(new MouseEvent('mouseenter')); + fixture.detectChanges(); + + expect(component.highlightedStatus).toBe('redo'); + expect(breakdown.classList).toContain('is-hover-highlighting'); + expect( + fixture.nativeElement.querySelector('.ppi-segment[data-status="redo"]').classList, + ).toContain('is-active'); + expect( + fixture.nativeElement.querySelector('.ppi-segment[data-status="complete"]').classList, + ).not.toContain('is-active'); + expect(fixture.nativeElement.querySelector('.ppi-tooltip').textContent.trim()).toBe( + 'Redo · 10%', + ); + + row.dispatchEvent(new MouseEvent('mouseleave')); + fixture.detectChanges(); + expect(component.highlightedStatus).toBeNull(); + expect(fixture.nativeElement.querySelector('.ppi-tooltip')).toBeNull(); + + row.dispatchEvent(new FocusEvent('focus')); + fixture.detectChanges(); + expect(component.highlightedStatus).toBe('redo'); + expect(breakdown.classList).toContain('is-focus-highlighting'); + expect(row.getAttribute('tabindex')).toBe('0'); + }); + + it('plays the entrance only for the first result, not on a refresh', () => { + load(of(NORMAL_STATE)); + expect(component.animateEntry).toBe(true); + expect(fixture.nativeElement.querySelector('.ppi-summary').classList).toContain('is-entering'); + + load(of(NORMAL_STATE)); + expect(component.animateEntry).toBe(false); + expect(fixture.nativeElement.querySelector('.ppi-summary').classList).not.toContain( + 'is-entering', + ); + }); + it.each([ {state: ROUNDED_90_STATE, total: 90}, {state: ROUNDED_110_STATE, total: 110}, diff --git a/src/app/projects/states/dashboard/directives/task-dashboard/directives/task-description-card/ppi-widget/ppi-widget.component.ts b/src/app/projects/states/dashboard/directives/task-dashboard/directives/task-description-card/ppi-widget/ppi-widget.component.ts index f38e0c3dbd..8464cbb6d6 100644 --- a/src/app/projects/states/dashboard/directives/task-dashboard/directives/task-description-card/ppi-widget/ppi-widget.component.ts +++ b/src/app/projects/states/dashboard/directives/task-dashboard/directives/task-description-card/ppi-widget/ppi-widget.component.ts @@ -17,6 +17,8 @@ import {TaskStatus, TaskStatusEnum} from 'src/app/api/models/task-status'; import {PeerProgressIndicatorService} from 'src/app/api/services/peer-progress-indicator.service'; import {PeerProgressDisplayPreferenceService} from 'src/app/common/services/peer-progress-display-preference.service'; +type PeerProgressHighlightSource = 'pointer' | 'focus'; + interface PeerProgressDisplaySegment { status: TaskStatusEnum; label: string; @@ -38,6 +40,16 @@ export class PpiWidgetComponent implements OnChanges, OnDestroy { view: PeerProgressViewModel = {state: 'loading', data: null, message: null}; advanced = false; + // The status whose segment is emphasised, and whether a pointer or keyboard + // focus asked for it. Hover emphasis is only styled on fine-pointer devices. + highlightedStatus: TaskStatusEnum | null = null; + highlightSource: PeerProgressHighlightSource | null = null; + + // Bars grow in only the first time a result is shown. Later refreshes change + // widths in place without replaying the entrance. + animateEntry = false; + + private hasShownResult = false; private activeRequest?: Subscription; private readonly statusDisplayIndex = new Map( TaskStatus.PEER_PROGRESS_DISPLAY_ORDER.map((status, index) => [status, index]), @@ -130,6 +142,55 @@ export class PpiWidgetComponent implements OnChanges, OnDestroy { return `Anonymous peer task status distribution: ${detail}`; } + get highlightedSegment(): PeerProgressDisplaySegment | null { + if (this.highlightedStatus === null) { + return null; + } + return ( + this.displaySegments.find((segment) => segment.status === this.highlightedStatus) ?? null + ); + } + + // Rows per legend column, so a two-column legend fills top to bottom in + // display order instead of leaving a lone item on a final row. + get legendRows(): number { + return Math.max(1, Math.ceil(this.displaySegments.length / 2)); + } + + get breakdownHeadingId(): string { + return `peer-progress-breakdown-${this.taskDef?.id ?? 'loading'}`; + } + + highlight(status: TaskStatusEnum, source: PeerProgressHighlightSource): void { + this.highlightedStatus = status; + this.highlightSource = source; + this.cdr.markForCheck(); + } + + clearHighlight(source: PeerProgressHighlightSource): void { + if (this.highlightSource !== source) { + return; + } + this.highlightedStatus = null; + this.highlightSource = null; + this.cdr.markForCheck(); + } + + // Horizontal centre of a segment in the stacked bar, used to place its tooltip. + segmentMidpoint(status: TaskStatusEnum): string { + const segments = this.displaySegments; + const total = this.distributionTotal || 1; + let offset = 0; + + for (const segment of segments) { + if (segment.status === status) { + return `${((offset + segment.percentage / 2) / total) * 100}%`; + } + offset += segment.percentage; + } + return '50%'; + } + get titleId(): string { return `peer-progress-title-${this.taskDef?.id ?? 'loading'}`; } @@ -182,6 +243,13 @@ export class PpiWidgetComponent implements OnChanges, OnDestroy { private setView(next: PeerProgressViewModel): void { this.view = next; + this.highlightedStatus = null; + this.highlightSource = null; + + if (next.state === 'success' || next.state === 'no-data') { + this.animateEntry = !this.hasShownResult; + this.hasShownResult = true; + } this.cdr.markForCheck(); } diff --git a/src/app/projects/states/dashboard/directives/task-dashboard/directives/task-overseer-report/submission-files-modal/submission-files-modal.component.html b/src/app/projects/states/dashboard/directives/task-dashboard/directives/task-overseer-report/submission-files-modal/submission-files-modal.component.html index 9ac9a8bcaf..6541f15c84 100644 --- a/src/app/projects/states/dashboard/directives/task-dashboard/directives/task-overseer-report/submission-files-modal/submission-files-modal.component.html +++ b/src/app/projects/states/dashboard/directives/task-dashboard/directives/task-overseer-report/submission-files-modal/submission-files-modal.component.html @@ -16,7 +16,7 @@

- +
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 17b78325f3..1158c09b62 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 @@ -15,6 +15,7 @@ @@ -24,7 +25,13 @@

{{ task?.statusLabel() }}

@for (trigger of triggers; track trigger) {
- + +
{{ trigger.label }}
 diff --git a/src/app/projects/states/dashboard/directives/task-dashboard/directives/task-status-card/task-status-card.component.scss b/src/app/projects/states/dashboard/directives/task-dashboard/directives/task-status-card/task-status-card.component.scss index aa29e7e791..f0cd5e57d9 100644 --- a/src/app/projects/states/dashboard/directives/task-dashboard/directives/task-status-card/task-status-card.component.scss +++ b/src/app/projects/states/dashboard/directives/task-dashboard/directives/task-status-card/task-status-card.component.scss @@ -74,6 +74,11 @@ min-height: 44px; background-color: color-mix(in srgb, var(--tsc) 12%, var(--ot-color-surface-raised)); box-shadow: none; + + // the icon takes the same status hue as the outline, not the global primary + .mat-icon { + color: var(--tsc); + } } ::ng-deep f-task-status-card .mat-mdc-card-actions .mat-mdc-outlined-button:not(:disabled):hover { @@ -86,7 +91,8 @@ align-items: flex-start; justify-content: space-between; gap: 0.5rem; - padding: 0 0.75rem 0.75rem; + // line the buttons up with the card text above and give them room to breathe + padding: 0.75rem 16px 16px; } .task-status-primary-actions { @@ -102,9 +108,12 @@ } } +// Size this through Material's own token. It derives the width, the height and +// the padding together, so the glyph stays in the middle of its hover circle. +// Setting width and height by hand leaves the padding at the 40px default and +// the icon sits up and to the left of the ripple. .task-status-actions > [mat-icon-button] { - width: 44px; - height: 44px; + --mat-icon-button-state-layer-size: 44px; flex: none; } diff --git a/src/app/projects/states/dashboard/directives/task-dashboard/directives/task-submission-card/task-submission-card.component.html b/src/app/projects/states/dashboard/directives/task-dashboard/directives/task-submission-card/task-submission-card.component.html index a33e29097d..28a92865e8 100644 --- a/src/app/projects/states/dashboard/directives/task-dashboard/directives/task-submission-card/task-submission-card.component.html +++ b/src/app/projects/states/dashboard/directives/task-dashboard/directives/task-submission-card/task-submission-card.component.html @@ -80,13 +80,13 @@

Your submissio } @if (canRegeneratePdf) { - } @if (task?.inSubmittedState()) { - diff --git a/src/app/projects/states/dashboard/directives/task-dashboard/task-dashboard.component.html b/src/app/projects/states/dashboard/directives/task-dashboard/task-dashboard.component.html index 69258ff67c..f66013a59d 100644 --- a/src/app/projects/states/dashboard/directives/task-dashboard/task-dashboard.component.html +++ b/src/app/projects/states/dashboard/directives/task-dashboard/task-dashboard.component.html @@ -55,6 +55,7 @@ + + diff --git a/src/app/tasks/modals/feedback-appeal-modal/feedback-appeal-modal.component.scss b/src/app/tasks/modals/feedback-appeal-modal/feedback-appeal-modal.component.scss index 28beb6a350..d658d28b66 100644 --- a/src/app/tasks/modals/feedback-appeal-modal/feedback-appeal-modal.component.scss +++ b/src/app/tasks/modals/feedback-appeal-modal/feedback-appeal-modal.component.scss @@ -1,36 +1,152 @@ +@use '../../../unit-hub/hub-card' as hub; + +@include hub.card-theme; + +:host { + display: block; + max-width: 100%; +} + +.review-dialog-title { + padding-inline: 24px; +} + .feedback-review-content { + display: flex; max-height: calc(100dvh - 10rem); + flex-direction: column; + gap: 16px; overflow: auto; + padding: 0 24px 8px; + color: var(--ot-color-text); +} + +// Task abbreviation and name under the title. +.review-context { + display: flex; + min-width: 0; + flex-wrap: wrap; + align-items: center; + gap: 8px; + margin-top: -4px; + font-size: 0.875rem; + line-height: 1.4; +} + +.review-context__name { + overflow-wrap: anywhere; + font-weight: 600; +} + +// Same chip as the extension dialog and the extension request card. +.review-chip { + display: inline-flex; + align-items: center; + padding: 2px 8px; + border-radius: var(--ot-radius-pill); + background: color-mix(in srgb, var(--ot-color-primary) 14%, transparent); + color: var(--ot-color-text); + font-size: 0.75rem; + font-weight: 600; + line-height: 1.4; + white-space: nowrap; +} + +.review-description { + margin: 0; + color: var(--ot-color-text-muted); + line-height: 1.5; +} + +.review-callout { + @include hub.callout(raised); + color: var(--ot-color-text); + + > mat-icon { + color: var(--ot-color-info); + } +} + +.review-callout__text { + display: flex; + flex-direction: column; + gap: 4px; + + p { + margin: 0; + } + + p + p { + color: var(--ot-color-text-muted); + } +} + +.review-field { + margin: 0; +} + +.review-counter { + color: var(--ot-color-text-muted); + font-variant-numeric: tabular-nums; +} + +.review-counter--warn { + color: var(--ot-color-warning); +} + +.review-counter--limit { + color: var(--ot-color-error); + font-weight: 600; } .feedback-review-actions { display: flex; flex-wrap: wrap; - justify-content: space-between; - gap: 0.5rem; - padding: 0.75rem 1.5rem 1rem; + justify-content: flex-end; + gap: 12px; + padding: 16px 24px 24px; button { min-height: 44px; + margin: 0; white-space: normal; } } +// The app paints every spinner circle in the divider colour, which vanishes on a +// disabled button. Inside the button it follows the label colour instead. +.review-submit__spinner { + display: inline-block; + margin-right: 8px; + vertical-align: middle; + + ::ng-deep circle { + stroke: currentColor !important; + } +} + .feedback-review-error { + margin: 0; border-left: 4px solid var(--ot-color-error); background: color-mix(in srgb, var(--ot-color-error) 10%, transparent); padding: 0.75rem; color: var(--ot-color-text); } -@media (max-width: 479.98px) { +@media (max-width: 599.98px) { + .review-dialog-title { + padding-inline: 16px; + } + .feedback-review-content { max-height: calc(100dvh - 12rem); - padding-inline: 1rem; + padding-inline: 16px; } .feedback-review-actions { - padding-inline: 1rem; + flex-direction: column-reverse; + align-items: stretch; + padding-inline: 16px; button { width: 100%; diff --git a/src/app/tasks/modals/feedback-appeal-modal/feedback-appeal-modal.component.spec.ts b/src/app/tasks/modals/feedback-appeal-modal/feedback-appeal-modal.component.spec.ts index 4ab64cb3ad..e8322ca9c7 100644 --- a/src/app/tasks/modals/feedback-appeal-modal/feedback-appeal-modal.component.spec.ts +++ b/src/app/tasks/modals/feedback-appeal-modal/feedback-appeal-modal.component.spec.ts @@ -1,5 +1,17 @@ -import {afterEach, describe, expect, it, vi} from 'vitest'; -import {MatDialog, MatDialogConfig, MatDialogRef} from '@angular/material/dialog'; +import {afterEach, beforeEach, describe, expect, it, vi} from 'vitest'; +import {ComponentFixture, TestBed} from '@angular/core/testing'; +import {FormsModule} from '@angular/forms'; +import {MatButtonModule} from '@angular/material/button'; +import { + MAT_DIALOG_DATA, + MatDialog, + MatDialogConfig, + MatDialogModule, + MatDialogRef, +} from '@angular/material/dialog'; +import {MatIconModule} from '@angular/material/icon'; +import {MatInputModule} from '@angular/material/input'; +import {MatProgressSpinnerModule} from '@angular/material/progress-spinner'; import {of, throwError} from 'rxjs'; import {Task} from 'src/app/api/models/task'; import {TaskService} from 'src/app/api/services/task.service'; @@ -12,6 +24,7 @@ function buildComponent(requestFeedbackReview = vi.fn(() => of({}))) { const addComment = vi.fn(); const task = { definition: {abbreviation: '1.1P', name: 'Hello World'}, + project: {escalationAttemptsRemaining: 3}, requestFeedbackReview, addComment, } as unknown as Task; @@ -42,6 +55,17 @@ describe('FeedbackAppealModalComponent', () => { expect(confirm).toHaveBeenCalled(); }); + it('turns the counter to warning at 900 characters and error at the limit', () => { + const {component} = buildComponent(); + + component.reviewComment = 'a'.repeat(899); + expect(component.commentCounterState).toBe('ok'); + component.reviewComment = 'a'.repeat(900); + expect(component.commentCounterState).toBe('warn'); + component.reviewComment = 'a'.repeat(1000); + expect(component.commentCounterState).toBe('limit'); + }); + it('submits trimmed text once and keeps controlled failures open', () => { const success = buildComponent(); success.component.reviewComment = ' Please review criterion one. '; @@ -65,6 +89,88 @@ describe('FeedbackAppealModalComponent', () => { }); }); +describe('FeedbackAppealModalComponent template', () => { + let fixture: ComponentFixture; + let component: FeedbackAppealModalComponent; + + beforeEach(async () => { + const {component: built} = buildComponent(); + await TestBed.configureTestingModule({ + declarations: [FeedbackAppealModalComponent], + imports: [ + FormsModule, + MatButtonModule, + MatDialogModule, + MatIconModule, + MatInputModule, + MatProgressSpinnerModule, + ], + providers: [ + {provide: MatDialogRef, useValue: {close: vi.fn()}}, + {provide: MAT_DIALOG_DATA, useValue: built.data}, + {provide: AlertService, useValue: {success: vi.fn(), error: vi.fn()}}, + {provide: TaskService, useValue: {notifyStatusChange: vi.fn()}}, + ], + }).compileComponents(); + + fixture = TestBed.createComponent(FeedbackAppealModalComponent); + component = fixture.componentInstance; + fixture.detectChanges(); + await fixture.whenStable(); + }); + + const el = () => fixture.nativeElement as HTMLElement; + const text = (selector: string) => + el().querySelector(selector)?.textContent?.replace(/\s+/g, ' ').trim(); + const submitButton = () => el().querySelector('button.review-submit'); + + it('renders the title, task context, description and allowance callout', () => { + expect(text('[mat-dialog-title]')).toBe('Request a feedback review'); + expect(text('.review-chip')).toBe('1.1P'); + expect(text('.review-context__name')).toBe('Hello World'); + expect(text('.review-description')).toBe( + 'Another tutor will reassess the feedback on this task and confirm or revise it.', + ); + expect(text('.review-callout__text')).toBe( + "You have 3 review requests left for this unit. If the feedback is revised, this one won't count. Review decisions are final once resolved.", + ); + expect(text('.review-callout b')).toBe('3 review requests'); + }); + + it('labels the primary action and enables it once a reason is entered', async () => { + expect(text('button.review-submit')).toBe('rate_review Submit review request'); + expect(submitButton()?.disabled).toBe(true); + + component.reviewComment = 'Section 3 covers the missing test cases.'; + fixture.detectChanges(); + await fixture.whenStable(); + fixture.detectChanges(); + + expect(submitButton()?.disabled).toBe(false); + }); + + it('marks the counter as a warning from 900 characters', async () => { + component.reviewComment = 'a'.repeat(900); + fixture.detectChanges(); + await fixture.whenStable(); + fixture.detectChanges(); + + expect(text('.review-counter')).toBe('900 / 1000'); + expect(el().querySelector('.review-counter')?.classList).toContain('review-counter--warn'); + }); + + it('shows a spinner and disables both actions while submitting', () => { + component.submitting = true; + fixture.detectChanges(); + + expect(el().querySelector('.review-submit__spinner')).not.toBeNull(); + const buttons = Array.from( + el().querySelectorAll('.feedback-review-actions button'), + ); + expect(buttons.every((button) => button.disabled)).toBe(true); + }); +}); + describe('FeedbackAppealModalService', () => { it('uses the shared responsive/focus-safe dialog contract', () => { const dialogRef = {}; @@ -78,7 +184,7 @@ describe('FeedbackAppealModalService', () => { autoFocus: 'dialog', closeOnNavigation: true, maxHeight: 'calc(100dvh - 2rem)', - maxWidth: '700px', + maxWidth: '560px', restoreFocus: true, width: 'calc(100vw - 2rem)', }); diff --git a/src/app/tasks/modals/feedback-appeal-modal/feedback-appeal-modal.component.ts b/src/app/tasks/modals/feedback-appeal-modal/feedback-appeal-modal.component.ts index 2f8a71de59..551eaad1cd 100644 --- a/src/app/tasks/modals/feedback-appeal-modal/feedback-appeal-modal.component.ts +++ b/src/app/tasks/modals/feedback-appeal-modal/feedback-appeal-modal.component.ts @@ -16,6 +16,10 @@ export class FeedbackAppealModalComponent implements OnInit { task: Task; reviewComment = ''; + /** Matches the maxlength on the textarea. */ + readonly commentMaxLength = 1000; + /** The counter turns to the warning colour from this many characters. */ + readonly commentWarnLength = 900; submitting = false; errorMessage = ''; private allowClose = false; @@ -60,6 +64,17 @@ export class FeedbackAppealModalComponent implements OnInit { }); } + public get commentLength(): number { + return this.reviewComment?.length ?? 0; + } + + public get commentCounterState(): 'ok' | 'warn' | 'limit' { + if (this.commentLength >= this.commentMaxLength) { + return 'limit'; + } + return this.commentLength >= this.commentWarnLength ? 'warn' : 'ok'; + } + public get isDirty(): boolean { return this.reviewComment.trim().length > 0; } diff --git a/src/app/tasks/modals/feedback-appeal-modal/feedback-appeal-modal.service.ts b/src/app/tasks/modals/feedback-appeal-modal/feedback-appeal-modal.service.ts index 21c94e0e17..08412e8d82 100644 --- a/src/app/tasks/modals/feedback-appeal-modal/feedback-appeal-modal.service.ts +++ b/src/app/tasks/modals/feedback-appeal-modal/feedback-appeal-modal.service.ts @@ -23,7 +23,7 @@ export class FeedbackAppealModalService { task: task, }, maxHeight: 'calc(100dvh - 2rem)', - maxWidth: '700px', + maxWidth: '560px', panelClass: 'responsive-task-dialog', restoreFocus: true, width: 'calc(100vw - 2rem)', diff --git a/src/app/tasks/modals/upload-submission-modal/task-upload-requirements/task-upload-requirements.component.html b/src/app/tasks/modals/upload-submission-modal/task-upload-requirements/task-upload-requirements.component.html index 19ed440ed1..9b168548fc 100644 --- a/src/app/tasks/modals/upload-submission-modal/task-upload-requirements/task-upload-requirements.component.html +++ b/src/app/tasks/modals/upload-submission-modal/task-upload-requirements/task-upload-requirements.component.html @@ -1,58 +1,73 @@
- @if (hasRequirements) { -

Files required: {{ requiredFileCount }}

+ -
    - @for (summary of summaries; track $index) { -
  • -

    - Required file type: {{ summary.categoryLabel }} -

    - @if (summary.name) { -

    {{ summary.name }}

    - } -

    - Accepted formats: - @if (summary.extensions.length) { - {{ summary.previewExtensions.join(', ') }} - @if (summary.hasMoreExtensions) { - , … +

    +

    What to upload

    + + @if (hasRequirements) { +
      +
    • + + {{ requiredFileCountLabel }} +
      • + + @for (summary of summaries; track $index) { +
    • + +
      +

      + {{ summary.categoryLabel }} + &ngsp;·&ngsp; + + @if (summary.extensions.length) { + @if (summary.hasMoreExtensions) { + {{ summary.previewExtensions.join(', ') }}, and more + } @else { + {{ formatList(summary.previewExtensions) }} + } + } @else { + Not specified for this task + } + +

      + + @if (summary.maxSizeLabel) { +

      Up to {{ summary.maxSizeLabel }}

      } - } @else { - Not specified for this task - } -

      -

      - Maximum size: Not provided by the server. Upload limits still apply. -

      - @if (summary.hasMoreExtensions) { - -
        - @for (extension of summary.extensions; track extension) { -
      • {{ extension }}
        • + @if (showDescription(summary)) { +

        {{ summary.name }}

        } -
      - } -
      • - } -
    - } @else { -

    - Upload requirements for this task are not currently available. Contact your teaching team if - this message persists. -

    - } + + @if (summary.hasMoreExtensions) { + +
      + @for (extension of summary.extensions; track extension) { +
    • {{ extension }}
      • + } +
    + } +
    +
    • + } +
+ } @else { +

+ Upload requirements for this task are not currently available. Contact your teaching team if + this message persists. +

+ } +

diff --git a/src/app/tasks/modals/upload-submission-modal/task-upload-requirements/task-upload-requirements.component.scss b/src/app/tasks/modals/upload-submission-modal/task-upload-requirements/task-upload-requirements.component.scss index 37a5354fe8..7aa51e7bd1 100644 --- a/src/app/tasks/modals/upload-submission-modal/task-upload-requirements/task-upload-requirements.component.scss +++ b/src/app/tasks/modals/upload-submission-modal/task-upload-requirements/task-upload-requirements.component.scss @@ -1,63 +1,122 @@ +@use '../../../../unit-hub/hub-card' as hub; + +@include hub.card-theme; + +// A quiet callout: info icon, then a heading and one icon row per fact. .task-upload-requirements { - margin-bottom: 1em; - margin-top: 1em; + @include hub.callout(raised); min-width: 0; } +.requirements-callout-icon { + flex-shrink: 0; +} + +.requirements-body { + display: flex; + min-width: 0; + flex: 1 1 auto; + flex-direction: column; + gap: 8px; +} + +.requirements-heading { + margin: 0; + color: var(--ot-color-text); + font-size: 0.95rem; + font-weight: 650; + line-height: 1.4; +} + .requirements-list { - list-style: none; + display: flex; + flex-direction: column; + gap: 6px; margin: 0; padding: 0; + list-style: none; +} + +.requirement-row { + display: flex; + min-width: 0; + align-items: flex-start; + gap: 10px; + color: var(--ot-color-text); + overflow-wrap: anywhere; + + > mat-icon { + flex-shrink: 0; + width: 18px; + height: 18px; + margin-top: 2px; + color: var(--ot-color-text-muted); + font-size: 18px; + } +} + +.requirement-text { + display: flex; + min-width: 0; + flex-direction: column; + gap: 2px; + + p { + margin: 0; + } } -.requirement { - padding: 0.5em 0.75em; - margin-bottom: 0.5em; - border: 1px solid #ddd; - border-radius: 4px; - word-break: break-word; +.requirement-label { + font-weight: 600; +} + +.requirement-separator { + color: var(--ot-color-text-muted); +} + +.requirement-name, +.requirement-max-size { + color: var(--ot-color-text-muted); } .requirements-missing { - color: inherit; - font-style: italic; + margin: 0; + color: var(--ot-color-text-muted); } .extensions-toggle { - background: none; - border: none; + align-self: flex-start; + margin-top: 2px; padding: 0; - color: inherit; - text-decoration: underline; + border: none; + border-radius: var(--ot-radius-xs); + background: none; + color: var(--ot-color-link); cursor: pointer; - font-size: inherit; + font: inherit; + font-weight: 600; + text-decoration: underline; + text-underline-offset: 3px; - &:hover, - &:focus { - outline: 2px solid currentColor; - outline-offset: 3px; + &:focus-visible { + outline: 2px solid var(--ot-color-focus); + outline-offset: 2px; } } .extensions-list { display: flex; flex-wrap: wrap; - gap: 0.25em 0.75em; - list-style: none; - margin: 0.5em 0 0; + gap: 4px 6px; + margin: 6px 0 0; padding: 0; + list-style: none; li { - border: 1px solid currentColor; - border-radius: 3px; - padding: 0.1em 0.5em; - } -} - -// Narrow screens: stack extension chips full-width instead of relying on wrapping alone -@media (max-width: 480px) { - .extensions-list { - flex-direction: column; - gap: 0.25em; + padding: 1px 8px; + border: 1px solid color-mix(in srgb, var(--ot-color-border) 45%, transparent); + border-radius: var(--ot-radius-pill); + color: var(--ot-color-text-muted); + font-size: 0.78rem; } } diff --git a/src/app/tasks/modals/upload-submission-modal/task-upload-requirements/task-upload-requirements.component.spec.ts b/src/app/tasks/modals/upload-submission-modal/task-upload-requirements/task-upload-requirements.component.spec.ts index 7dce66a720..49696713c1 100644 --- a/src/app/tasks/modals/upload-submission-modal/task-upload-requirements/task-upload-requirements.component.spec.ts +++ b/src/app/tasks/modals/upload-submission-modal/task-upload-requirements/task-upload-requirements.component.spec.ts @@ -1,4 +1,5 @@ import {ComponentFixture, TestBed} from '@angular/core/testing'; +import {MatIconModule} from '@angular/material/icon'; import {UploadRequirement} from 'src/app/api/models/task-definition'; import {TaskUploadRequirementsComponent} from './task-upload-requirements.component'; @@ -8,6 +9,7 @@ describe('TaskUploadRequirementsComponent', () => { beforeEach(async () => { await TestBed.configureTestingModule({ + imports: [MatIconModule], declarations: [TaskUploadRequirementsComponent], }).compileComponents(); @@ -41,8 +43,8 @@ describe('TaskUploadRequirementsComponent', () => { ]); const text = (fixture.nativeElement as HTMLElement).textContent; - expect(text).toContain('Files required:'); - expect(text).toContain('2'); + expect(text).toContain('What to upload'); + expect(text).toContain('2 files required'); expect(text).toContain('Document'); expect(text).toContain('PDF'); expect(text).toContain('Spreadsheet'); @@ -51,6 +53,22 @@ describe('TaskUploadRequirementsComponent', () => { expect(text).toContain('XLSX'); }); + it('reads as icon rows: count, then type and formats, then a differing description', () => { + setRequirements([{key: 'file0', name: 'Demo document', type: 'document'}]); + + const text = (fixture.nativeElement as HTMLElement).textContent!.replace(/\s+/g, ' '); + expect(text).toContain('1 file required'); + expect(text).toContain('Document · PDF or PS'); + expect(text).toContain('Demo document'); + expect(fixture.nativeElement.querySelectorAll('.requirement-row mat-icon').length).toBe(2); + }); + + it('hides the description when it only repeats the type name', () => { + setRequirements([{key: 'file0', name: 'Document', type: 'document'}]); + + expect(fixture.nativeElement.querySelector('.requirement-name')).toBeNull(); + }); + it('does not show an expand control when the extension list is short', () => { setRequirements([{key: 'file0', name: 'Report', type: 'document'}]); @@ -101,8 +119,20 @@ describe('TaskUploadRequirementsComponent', () => { setRequirements([{key: 'file0', name: 'Report', type: 'document'}]); const text = (fixture.nativeElement as HTMLElement).textContent; - expect(text).toContain('Maximum size:'); - expect(text).toContain('Not provided by the server'); + expect(component.summaries[0].maxSizeLabel).toBeNull(); + expect(fixture.nativeElement.querySelector('.requirement-max-size')).toBeNull(); + expect(text).not.toContain('Not provided by the server'); + expect(text).not.toMatch(/\d+\s?(KB|MB|GB)/); + }); + + it('shows a size limit only when one is provided', () => { + setRequirements([{key: 'file0', name: 'Report', type: 'document'}]); + component.summaries[0].maxSizeLabel = '10 MB'; + fixture.detectChanges(); + + expect(fixture.nativeElement.querySelector('.requirement-max-size').textContent).toContain( + '10 MB', + ); }); }); diff --git a/src/app/tasks/modals/upload-submission-modal/task-upload-requirements/task-upload-requirements.component.ts b/src/app/tasks/modals/upload-submission-modal/task-upload-requirements/task-upload-requirements.component.ts index 3e9652f0e5..680de9cd8a 100644 --- a/src/app/tasks/modals/upload-submission-modal/task-upload-requirements/task-upload-requirements.component.ts +++ b/src/app/tasks/modals/upload-submission-modal/task-upload-requirements/task-upload-requirements.component.ts @@ -43,6 +43,25 @@ export class TaskUploadRequirementsComponent implements OnChanges { return this.summaries.length; } + public get requiredFileCountLabel(): string { + const count = this.requiredFileCount; + return `${count} ${count === 1 ? 'file' : 'files'} required`; + } + + /** Joins a list as "A, B or C". */ + public formatList(items: readonly string[]): string { + if (items.length <= 1) { + return items.join(''); + } + return `${items.slice(0, -1).join(', ')} or ${items[items.length - 1]}`; + } + + /** The file's own description, only when it says more than the type name does. */ + public showDescription(summary: UploadRequirementSummary): boolean { + const name = summary.name?.trim().toLowerCase(); + return !!name && name !== summary.categoryLabel.toLowerCase(); + } + public isExpanded(summary: UploadRequirementSummary): boolean { return this.expandedKeys.has(summary.key); } diff --git a/src/app/tasks/modals/upload-submission-modal/task-upload-requirements/upload-category.ts b/src/app/tasks/modals/upload-submission-modal/task-upload-requirements/upload-category.ts index 94a219e580..c776c6a629 100644 --- a/src/app/tasks/modals/upload-submission-modal/task-upload-requirements/upload-category.ts +++ b/src/app/tasks/modals/upload-submission-modal/task-upload-requirements/upload-category.ts @@ -9,12 +9,25 @@ const CATEGORY_LABELS: Record = { zip: 'Archive', }; +const CATEGORY_ICONS: Record = { + document: 'description', + csv: 'table_chart', + code: 'code', + image: 'image', + zip: 'folder_zip', +}; + export const EXTENSION_PREVIEW_LIMIT = 8; export interface UploadRequirementSummary { key: string; name: string; categoryLabel: string; + icon: string; + // The API does not expose a per-file size limit today. When it does, set this + // and the requirement row shows it. Until then nothing is shown, so no limit is + // invented for the student. + maxSizeLabel: string | null; extensions: string[]; previewExtensions: string[]; hasMoreExtensions: boolean; @@ -34,6 +47,8 @@ export function summariseUploadRequirement( categoryLabel: knownType ? CATEGORY_LABELS[type as keyof typeof ACCEPTED_TYPES] : type || 'File', + icon: knownType ? CATEGORY_ICONS[type as keyof typeof ACCEPTED_TYPES] : 'insert_drive_file', + maxSizeLabel: null, extensions, previewExtensions: extensions.slice(0, EXTENSION_PREVIEW_LIMIT), hasMoreExtensions: extensions.length > EXTENSION_PREVIEW_LIMIT, diff --git a/src/app/tasks/modals/upload-submission-modal/upload-submission-modal.component.html b/src/app/tasks/modals/upload-submission-modal/upload-submission-modal.component.html index b6b36b6ef4..833239fbcd 100644 --- a/src/app/tasks/modals/upload-submission-modal/upload-submission-modal.component.html +++ b/src/app/tasks/modals/upload-submission-modal/upload-submission-modal.component.html @@ -1,216 +1,351 @@ -
-

Submit {{ task.definition.abbreviation }} {{ task.definition.name }}

- - Submission type - - @for (option of submissionTypeOptions; track option.id) { - {{ option.label }} + +@if (!uploadStarted) { +
+

Submit task

+
+ {{ task.definition.abbreviation }} + {{ task.definition.name }} + @if (dueDate) { + + + Due {{ dueDate | date: 'EEE d MMM' }} + } - - -
+
+ +} - -
- @if (isGroupStage && !uploadStarted) { - - -
-

Team Contribution

-

- Please rate each team member's effort on {{ task.definition.name }}. -

-

Click the same rating twice to record no contribution.

-
+ + + @if (showSubmitFlow) { +
+ + - {{ flowTitle }}

+

{{ flowDetail }}

+ +
+ +
+ + + +
+ } + + +
+ + @if (!uploadStarted) { +
+ Submit as + + + + @if (statusFor(submissionType); as status) { + + } @else { + + } + {{ selectedSubmissionTypeLabel }} + + + @for (option of submissionTypeOptions; track option.id) { + + + @if (statusFor(option.id); as status) { + + } @else { + + } + {{ option.label }} + + + } + +
+ } + + @if (steps.length > 1 && !uploadStarted) { +
    + @for (step of steps; track step.stage) { +
  1. - - - + + {{ step.label }} + @if ($index < currentStepIndex) { + (done) + } +
    2. + } +
} + @if (isGroupStage && !uploadStarted) { +
+
+

Team contribution

+

+ Please rate each team member's effort on {{ task.definition.name }}. +

+

+ Click the same rating twice to record no contribution. +

+
- - - @if (!uploadStarted) { -
-

- Select {{ submissionType === 'need_help' ? 'files' : 'evidence' }} to upload -

- - @if (submissionType === 'need_help') { -

- Upload the required files so your tutor can help with this task, even if they are - not finished yet. + + +

+ } + +
+ @if (!uploadStarted && submissionType === 'need_help') { +

+ Upload the required files so your tutor can help with this task, even if they are not + finished yet. +

+ } + + + + + + +
+ + @if (!uploadStarted && isCommentsStage) { +
+
+ @if (submissionType === 'need_help') { +

What do you need help with?

+

+ Add a short comment so your tutor knows what help you need on this task. +

+ } @else { +

Final comments

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

+ Please specify which areas of your submission you would like feedback on.

} @else { -

- Upload files that show your evidence of work until each required file has been - selected. +

+ Add an optional note for your tutor to read while assessing this submission.

} + } +
+ + + Comment + + + +
+ Character count: {{ comment.length }} + @if (requiresComment) { + (Min. {{ minCommentLength }}) + } +
+
+ + @if (privacyPolicy.privacy) { +
+
+

Declaration

+

{{ privacyPolicy.privacy }}

- } - - - - - - +
+ + - @if (!uploadStarted && isCommentsStage) { -
- - -
- @if (submissionType === 'need_help') { -

What do you need help with?

-

- Add a short comment so your tutor knows what help you need on this task. -

- } @else { -

Final comments

- @if (task.definition.assessInPortfolioOnly) { -

- Please specify which areas of your submission you would like feedback on. -

- } @else { -

- Add an optional note for your tutor to read while assessing this submission. -

- } - } -
- - - Comment - - - -
- Character count: {{ comment.length }} - @if (requiresComment) { - (Min. {{ minCommentLength }}) - } -
- - - - @if (privacyPolicy.privacy) { - - -
-

Declaration

-

{{ privacyPolicy.privacy }}

-
- -
- - - @if (showPlagiarism) { -
- {{ privacyPolicy.plagiarism }} -
- } -
- - - } -
+ @if (showPlagiarism) { +

+ {{ privacyPolicy.plagiarism }} +

+ } +
+
+ } }
-@if (!uploadStarted) { - - - @if (isGroupStage) { - - } +
+ @if (continueHint; as hint) { + {{ hint }} + } - @if (isDetailsStage && showCommentsSection) { - - } + @if (isDetailsStage && showGroupSection) { + + } - @if (isDetailsStage && showGroupSection) { - - } + @if (isCommentsStage) { + + } - @if (isCommentsStage) { - - } + @if (isGroupStage) { + + } - @if ((!showCommentsSection && isDetailsStage) || isCommentsStage) { - - } + @if (isDetailsStage && showCommentsSection) { + + } + + @if ((!showCommentsSection && isDetailsStage) || isCommentsStage) { + + } +
} diff --git a/src/app/tasks/modals/upload-submission-modal/upload-submission-modal.component.reviewed.spec.ts b/src/app/tasks/modals/upload-submission-modal/upload-submission-modal.component.reviewed.spec.ts index 7e2343867f..6ada34b207 100644 --- a/src/app/tasks/modals/upload-submission-modal/upload-submission-modal.component.reviewed.spec.ts +++ b/src/app/tasks/modals/upload-submission-modal/upload-submission-modal.component.reviewed.spec.ts @@ -58,7 +58,7 @@ describe('UploadSubmissionModalComponent upload guidance', () => { const button = root.querySelector('.file-drop-zone')!; const input = root.querySelector('input[type=file]')!; const help = root.querySelector('.task-upload-requirements')!; - expect(help.textContent).toContain('Files required: 1'); + expect(help.textContent).toContain('1 file required'); expect(button.getAttribute('aria-describedby')).toBe(help.id); expect(input.getAttribute('aria-describedby')).toBe(help.id); expect(help.compareDocumentPosition(button) & Node.DOCUMENT_POSITION_FOLLOWING).toBeTruthy(); diff --git a/src/app/tasks/modals/upload-submission-modal/upload-submission-modal.component.scss b/src/app/tasks/modals/upload-submission-modal/upload-submission-modal.component.scss index 446858ca71..bf4561c85c 100644 --- a/src/app/tasks/modals/upload-submission-modal/upload-submission-modal.component.scss +++ b/src/app/tasks/modals/upload-submission-modal/upload-submission-modal.component.scss @@ -1,77 +1,607 @@ +// Submit task dialog: standard title, content and actions with 24px padding and no +// nested elevated cards. Colours come only from --ot-* tokens. + +// --- Header --- + .submission-dialog-header { - align-items: start; - display: grid; - gap: 1rem; - grid-template-columns: minmax(0, 1fr) minmax(16rem, 24rem); - padding-inline: 1.5rem; + display: flex; + min-width: 0; + flex-direction: column; + gap: 6px; + padding: 20px 24px 12px; +} + +:host .submission-dialog-title { + margin: 0; + padding: 0; + color: var(--ot-color-text); + font-size: 1.25rem; + font-weight: 650; + line-height: 1.35; + + // Material draws a spacer before the title; the header padding does that job. + &::before { + display: none; + } } -.submission-dialog-header h2 { +.submission-task-line { + display: flex; min-width: 0; - overflow-wrap: anywhere; - padding-inline: 0; + flex-wrap: wrap; + align-items: center; + gap: 4px 10px; + color: var(--ot-color-text); + font-size: 0.9rem; + line-height: 1.4; +} + +.submission-task-chip { + flex-shrink: 0; + padding: 1px 8px; + border: 1px solid color-mix(in srgb, var(--ot-color-link) 35%, transparent); + border-radius: var(--ot-radius-pill); + background: color-mix(in srgb, var(--ot-color-link) 10%, transparent); + color: var(--ot-color-text); + font-size: 0.75rem; + font-weight: 650; + letter-spacing: 0.02em; } -.submission-type-field { - margin-top: 0.75rem; +.submission-task-name { min-width: 0; - width: 100%; + font-weight: 600; + overflow-wrap: anywhere; +} + +.submission-task-due { + display: inline-flex; + align-items: center; + gap: 4px; + color: var(--ot-color-text-muted); + font-size: 0.82rem; + white-space: nowrap; + + mat-icon { + width: 16px; + height: 16px; + font-size: 16px; + } } -:host mat-dialog-content { +.submission-task-due--past { + color: var(--ot-urgency-overdue); +} + +// --- Content --- + +:host .submission-dialog-content { max-height: min(72dvh, 48rem); min-width: 0; + padding: 4px 24px 24px; overscroll-behavior: contain; } -.submission-dialog-actions { - gap: 0.5rem; - min-height: 64px; +.submission-dialog-body { + display: flex; + min-width: 0; + flex-direction: column; + gap: 16px; + color: var(--ot-color-text); } -.cancel-action { - margin-right: auto; +// "Submit as" sits on its own row: a label, then a compact themed picker. +.submission-type-row { + display: flex; + min-width: 0; + flex-wrap: wrap; + align-items: center; + gap: 8px 12px; + padding-top: 4px; } -@media (max-width: 639.98px) { - .submission-dialog-header { - display: flex; - flex-direction: column; - gap: 0; - padding-inline: 1rem; +.submission-type-label { + color: var(--ot-color-text); + font-size: 0.9rem; + font-weight: 600; +} + +.submission-type-select { + display: flex; + box-sizing: border-box; + width: min(100%, 18rem); + height: 44px; + align-items: center; + padding: 0 12px 0 8px; + border: 1px solid var(--ot-color-control-border); + border-radius: var(--ot-radius-md); + background-color: var(--ot-color-surface); + color: var(--ot-color-text); + font-size: 0.9rem; + + --mat-select-enabled-trigger-text-color: var(--ot-color-text); + --mat-select-placeholder-text-color: var(--ot-color-text-muted); + --mat-select-disabled-trigger-text-color: var(--ot-color-disabled-text); + --mat-select-enabled-arrow-color: var(--ot-color-text-muted); + --mat-select-focused-arrow-color: var(--ot-color-link); + + &:focus-visible, + &.mat-mdc-select-focused { + outline: none; + border-color: var(--ot-color-focus); + box-shadow: 0 0 0 1px var(--ot-color-focus); } - .submission-dialog-header h2 { - font-size: 1.25rem; - line-height: 1.3; - padding-bottom: 0; + &.mat-mdc-select-disabled { + border-color: color-mix(in srgb, var(--ot-color-border) 50%, transparent); + background-color: transparent; } +} + +.submission-type-option { + display: flex; + min-width: 0; + align-items: center; + gap: 8px; +} + +.submission-type-option__glyph { + flex: none; + width: 24px; + height: 20px; + color: var(--ot-color-link); + font-size: 20px; +} + +// status-icon is a 36px circle by default, sized by an important size-9 utility in the +// utilities layer, which no component rule can outrank. Scaling the spacing unit on the +// icon's host turns that same utility into a 24px circle in the picker and its menu. +.submission-type-option__icon { + --spacing: calc(24px / 9); + display: inline-flex; + flex: none; + + ::ng-deep .status-chip mat-icon { + width: 14px; + height: 14px; + font-size: 14px; + } +} + +::ng-deep .submission-type-panel { + --mat-select-panel-background-color: var(--ot-color-surface-raised); + --mat-option-label-text-color: var(--ot-color-text); + --mat-option-hover-state-layer-color: var(--ot-color-hover); + --mat-option-selected-state-layer-color: var(--ot-color-selected); + --mat-option-selected-state-label-text-color: var(--ot-color-text); + border: 1px solid var(--ot-color-divider); + border-radius: var(--ot-radius-md) !important; +} + +// --- Steps --- + +.submission-steps { + display: flex; + flex-wrap: wrap; + align-items: center; + gap: 6px 4px; + margin: 0; + padding: 0; + list-style: none; +} + +.submission-step { + display: inline-flex; + align-items: center; + gap: 6px; + color: var(--ot-color-text-muted); + font-size: 0.84rem; + line-height: 1.4; - .submission-type-field { - margin-top: 0.25rem; + // a thin connector between steps + & + &::before { + content: ''; + width: 20px; + height: 1px; + margin-inline: 4px 2px; + background: color-mix(in srgb, var(--ot-color-border) 60%, transparent); } +} - :host mat-dialog-content { - max-height: calc(100dvh - 13rem); - padding-inline: 0.75rem; +.submission-step__marker { + display: inline-flex; + width: 22px; + height: 22px; + flex: none; + align-items: center; + justify-content: center; + box-sizing: border-box; + border: 1px solid color-mix(in srgb, var(--ot-color-border) 70%, transparent); + border-radius: var(--ot-radius-circle); + font-size: 0.75rem; + font-weight: 650; + + mat-icon { + width: 14px; + height: 14px; + font-size: 14px; } +} - .submission-dialog-actions { - align-items: stretch; - display: grid; - grid-template-columns: repeat(2, minmax(0, 1fr)); - padding: 0.75rem; +.submission-step--current { + color: var(--ot-color-text); + font-weight: 600; + + .submission-step__marker { + border-color: var(--ot-color-primary); + background: var(--ot-color-primary); + color: var(--ot-color-on-primary); } +} - .submission-dialog-actions button { - margin: 0; +.submission-step--done .submission-step__marker { + border-color: color-mix(in srgb, var(--ot-color-link) 45%, transparent); + background: color-mix(in srgb, var(--ot-color-link) 12%, transparent); + color: var(--ot-color-link); +} + +// --- Sections --- + +.submission-section { + display: flex; + min-width: 0; + flex-direction: column; + gap: 12px; +} + +.submission-section__heading { + margin: 0 0 2px; + color: var(--ot-color-text); + font-size: 1rem; + font-weight: 650; + line-height: 1.4; +} + +.submission-section__lead { + margin: 0; + color: var(--ot-color-text-muted); + font-size: 0.9rem; + line-height: 1.5; +} + +.submission-character-count { + margin-top: -4px; + color: var(--ot-color-text-muted); + font-size: 0.82rem; + text-align: right; +} + +.submission-declaration { + padding-top: 16px; + border-top: 1px solid var(--ot-color-divider); +} + +// --- Actions --- + +:host .submission-dialog-actions { + display: flex; + min-height: 0; + flex-wrap: wrap; + align-items: center; + justify-content: space-between; + gap: 8px 12px; + padding: 16px 24px; + border-top: 1px solid var(--ot-color-divider); + + button { min-height: 44px; - min-width: 0; - white-space: normal; + margin: 0; + } +} + +.submission-dialog-actions__forward { + display: flex; + min-width: 0; + flex-wrap: wrap; + align-items: center; + justify-content: flex-end; + gap: 8px; + margin-left: auto; +} + +.submission-continue-hint { + color: var(--ot-color-text-muted); + font-size: 0.82rem; + line-height: 1.4; + text-align: right; +} + +// The cancel discards the selection, so it keeps an error-toned label on the outline. +:host .cancel-action:not(:disabled) { + --mat-button-outlined-label-text-color: color-mix( + in srgb, + var(--ot-color-error) 85%, + var(--ot-color-text) + ); + // Against the decorative border this outline is 2.21:1 on the dialog surface + // in dark, under the 3:1 WCAG asks of a control edge. The control token takes + // it to 3.50:1 and leaves light mode exactly where it was. + --mat-button-outlined-outline-color: color-mix( + in srgb, + var(--ot-color-error) 40%, + var(--ot-color-control-border) + ); + background-color: transparent; +} + +:host .cancel-action:not(:disabled):hover { + background-color: color-mix(in srgb, var(--ot-color-error) 7%, transparent); +} + +// --- Phones: the dialog fills the screen --- + +@media (max-width: 599.98px) { + ::ng-deep .cdk-overlay-pane.responsive-submission-dialog { + width: 100vw !important; + max-width: 100vw !important; + height: 100dvh; + max-height: 100dvh !important; + + .mat-mdc-dialog-surface { + border-radius: 0; + } + } + + .submission-dialog-header { + padding: 16px 16px 8px; + } + + :host .submission-dialog-content { + max-height: none; + flex: 1 1 auto; + padding: 4px 16px 16px; + } + + .submission-type-select { + width: 100%; + } + + :host .submission-dialog-actions { + padding: 12px 16px; + } + + .submission-dialog-actions__forward { + width: 100%; + + button { + flex: 1 1 0; + } + } + + .submission-continue-hint { + width: 100%; + text-align: left; } .cancel-action { - grid-column: 1 / -1; - grid-row: 2; + order: 2; + width: 100%; + } +} + +// --- The submit panel --- +// One panel from pressing Submit to the confirmation, and every part below is +// the same element the whole way through. The handover is the "collapse" shape +// from /submit-motion: the bar draws in to nothing, the circle takes the hit and +// answers it, and the mark draws out of the circle. Bar and mark read as one +// object rather than two things trading places. + +$flow-ease-out: cubic-bezier(0.23, 1, 0.32, 1); +$flow-ease-in-out: cubic-bezier(0.77, 0, 0.175, 1); + +.submit-flow { + display: flex; + flex-direction: column; + align-items: center; + gap: 8px; + min-height: 360px; + padding: 40px 0; + text-align: center; +} + +// Holds the confirmation's full height from the very first frame, so the circle +// growing inside it never pushes the words below it down. +.submit-flow__mark { + display: inline-flex; + align-items: center; + justify-content: center; + height: 112px; +} + +.submit-flow__ring { + position: relative; + display: inline-flex; + align-items: center; + justify-content: center; + width: 64px; + height: 64px; + border-radius: var(--ot-radius-circle, 999px); + background: color-mix(in srgb, var(--ot-color-primary) 14%, transparent); + color: var(--ot-color-primary); + transition: + width 380ms $flow-ease-in-out, + height 380ms $flow-ease-in-out, + background-color 300ms ease, + color 300ms ease; +} + +.submit-flow__glyph { + width: 28px; + height: 28px; + font-size: 28px; + transition: opacity 180ms ease; +} + +.submit-flow__title { + margin: 14px 0 0; + color: var(--ot-color-text); + font-size: 1.05rem; + font-weight: 620; + line-height: 1.3; +} + +.submit-flow__detail { + max-width: 36ch; + margin: 0; + color: var(--ot-color-text-muted); + font-size: 0.9rem; + line-height: 1.5; + overflow-wrap: anywhere; +} + +.submit-flow__title, +.submit-flow__detail, +.submit-flow__action { + transition: + opacity 180ms ease, + transform 200ms $flow-ease-out; +} + +.submit-flow__track { + position: relative; + overflow: hidden; + width: min(360px, 100%); + height: 8px; + margin-block-start: 14px; + border-radius: 999px; + background: var(--ot-color-surface-raised); + // Narrows into the middle first, then folds its own height away. It never just + // disappears: something vanishing is what reads as a new screen. + transition: + width 300ms $flow-ease-in-out, + height 200ms $flow-ease-out 260ms, + margin 200ms $flow-ease-out 260ms, + opacity 160ms ease 280ms; +} + +.submit-flow__fill { + position: absolute; + inset: 0; + border-radius: inherit; + background: var(--ot-color-primary); + // Clipped rather than sized, so each progress event stays on the compositor. + clip-path: inset(0 calc(100% - var(--flow-progress, 0%)) 0 0); + transition: + clip-path 260ms $flow-ease-out, + background-color 300ms ease; +} + +.submit-flow__action { + margin-block-start: 18px; + + &:active { + transform: scale(0.97); + } +} + +// The bytes are away. This lands before the confirmation is worked out, so the +// wait on the server is part of the sequence instead of dead time. +.submit-flow--done { + .submit-flow__ring { + background: color-mix(in srgb, var(--ot-color-success) 16%, transparent); + color: var(--ot-color-success); + } + + .submit-flow__fill { + background: var(--ot-color-success); + } +} + +// The handover itself. +.submit-flow--swapping { + .submit-flow__track { + width: 0; + } + + .submit-flow__ring { + animation: submit-flow-absorb 320ms $flow-ease-out 240ms both; + } + + .submit-flow__glyph, + .submit-flow__title, + .submit-flow__detail, + .submit-flow__action { + opacity: 0; + transform: translateY(6px); + } +} + +.submit-flow--celebrating { + .submit-flow__ring { + width: 104px; + height: 104px; + background: transparent; + } + + .submit-flow__track { + width: 0; + height: 0; + margin-block-start: 0; + opacity: 0; + } + + .submit-flow__title { + max-width: 20ch; + margin-block-start: 20px; + font-size: clamp(1.5rem, 4.5vw, 2rem); + font-weight: 680; + line-height: 1.2; + letter-spacing: -0.02em; + animation: submit-flow-in 300ms $flow-ease-out 160ms both; + } + + .submit-flow__detail { + animation: submit-flow-in 300ms $flow-ease-out 240ms both; + } + + .submit-flow__action { + animation: submit-flow-in 300ms $flow-ease-out 340ms both; + } +} + +@keyframes submit-flow-absorb { + 0% { + transform: scale(1); + } + + 45% { + transform: scale(1.07); + } + + 100% { + transform: scale(1); + } +} + +@keyframes submit-flow-in { + from { + opacity: 0; + transform: translateY(8px); + } + + to { + opacity: 1; + transform: none; + } +} + +@media (prefers-reduced-motion: reduce) { + .submit-flow :is(.submit-flow__ring, .submit-flow__glyph, .submit-flow__title), + .submit-flow :is(.submit-flow__detail, .submit-flow__track, .submit-flow__fill), + .submit-flow__action { + transition: none; + animation: none; + opacity: 1; + transform: none; } } diff --git a/src/app/tasks/modals/upload-submission-modal/upload-submission-modal.component.spec.ts b/src/app/tasks/modals/upload-submission-modal/upload-submission-modal.component.spec.ts index 806e0131d0..f4fccf526c 100644 --- a/src/app/tasks/modals/upload-submission-modal/upload-submission-modal.component.spec.ts +++ b/src/app/tasks/modals/upload-submission-modal/upload-submission-modal.component.spec.ts @@ -1,4 +1,4 @@ -import {beforeEach, describe, expect, it, vi} from 'vitest'; +import {type Mock, beforeEach, describe, expect, it, vi} from 'vitest'; import {Task} from 'src/app/api/models/task'; import { UploadSubmissionModalComponent, @@ -57,6 +57,7 @@ describe('UploadSubmissionModalComponent', () => { (component as unknown as {fileUploader: unknown}).fileUploader = { isUploading: false, + uploadInFlight: false, hasSelectedFiles: () => true, }; const confirm = vi.spyOn(window, 'confirm').mockReturnValue(false); @@ -73,6 +74,7 @@ describe('UploadSubmissionModalComponent', () => { it('cannot dismiss while the upload request is active', () => { (component as unknown as {fileUploader: unknown}).fileUploader = { isUploading: true, + uploadInFlight: true, hasSelectedFiles: () => true, }; const confirm = vi.spyOn(window, 'confirm'); @@ -82,6 +84,97 @@ describe('UploadSubmissionModalComponent', () => { confirm.mockRestore(); }); + it('can be dismissed and retried once a failed upload has settled', () => { + // The uploader keeps isUploading set after the request lands, because its + // own outcome panels render under it. Reading that as "still uploading" + // trapped the student on the error: no backdrop, no Escape, no retry. + (component as unknown as {fileUploader: unknown}).fileUploader = { + isUploading: true, + uploadInFlight: false, + uploadingInfo: {complete: true, success: false}, + hasSelectedFiles: () => true, + }; + component.uploadStarted = true; + // What the uploader's onFailure callback does, rather than a hand-set: the + // Submit button stays locked without it and the retry below is unreachable. + component.onUploadFailed(); + + expect(component.uploadFailed).toBe(true); + + const confirm = vi.spyOn(window, 'confirm').mockReturnValue(true); + expect(component.canClose()).toBe(true); + expect(confirm).toHaveBeenCalled(); + confirm.mockRestore(); + + const startUpload = vi.fn(); + component.onUploaderReady(startUpload); + component.uploadButtonClicked(); + expect(startUpload).toHaveBeenCalled(); + }); + + it('never asks about discarding work the server has already taken', () => { + // canClose is the dialog's closePredicate, and Material runs it for + // programmatic closes too. Prompting here both lied to the student and + // vetoed the dialog closing itself: answering "Cancel" to "discard your + // files?" kept them in it. + (component as unknown as {fileUploader: unknown}).fileUploader = { + isUploading: true, + uploadInFlight: false, + hasSelectedFiles: () => true, + }; + const confirm = vi.spyOn(window, 'confirm'); + + component.onUploadSuccess({id: 8, project_id: 1, status: 'ready_for_feedback'}); + + // The response is in, the confirmation has not been worked out yet. + expect(component.celebration).toBeNull(); + expect(component.canClose()).toBe(true); + expect(confirm).not.toHaveBeenCalled(); + confirm.mockRestore(); + }); + + it('records the submission when the dialog is dismissed before the callback', () => { + // Escape or the backdrop on the "Uploaded" panel destroys the dialog before + // the uploader reports completion. The bytes are already accepted, so the + // status change still has to land, with the celebration left for elsewhere. + component.submissionType = 'ready_for_feedback'; + component.onUploadSuccess({id: 8, project_id: 1, status: 'ready_for_feedback'}); + + component.ngOnDestroy(); + + expect(task.updateFromJson).toHaveBeenCalled(); + expect(task.processTaskStatusChange).toHaveBeenCalledWith( + 'ready_for_feedback', + expect.anything(), + true, + false, + ); + }); + + it('still records the status change when Done is pressed before the callback fires', () => { + // Done is on screen from the moment the bytes land, which is about a second + // before the uploader reports completion. Leaving in that window used to + // close the dialog without ever applying the response. + component.submissionType = 'ready_for_feedback'; + component.onUploadSuccess({id: 8, project_id: 1, status: 'ready_for_feedback'}); + + component.finishCelebration(); + + expect(task.updateFromJson).toHaveBeenCalled(); + // Unclaimed, so the dashboard gets to show what this dialog no longer will. + expect(task.processTaskStatusChange).toHaveBeenCalledWith( + 'ready_for_feedback', + expect.anything(), + true, + false, + ); + expect(dialogRef.close).toHaveBeenCalledWith({value: task}); + + // And the uploader's own callback must not apply it a second time. + component.onUploadComplete(); + expect(task.processTaskStatusChange).toHaveBeenCalledTimes(1); + }); + it('marks the task queued immediately and closes on the same task after upload', () => { component.submissionType = 'ready_for_feedback'; component.onUploadSuccess({id: 8, project_id: 1, status: 'ready_for_feedback'}); @@ -96,10 +189,95 @@ describe('UploadSubmissionModalComponent', () => { 'ready_for_feedback', expect.anything(), true, + true, ); expect(dialogRef.close).toHaveBeenCalledWith({value: task}); }); + it('holds the dialog open on the confirmation, then closes on the same task', () => { + vi.useFakeTimers(); + (task.processTaskStatusChange as unknown as Mock).mockReturnValue({ + timing: 'on_time', + resubmission: false, + tone: 'success', + particles: true, + headline: 'Submitted on time. Ready for feedback', + detail: 'T1 A task', + }); + + component.submissionType = 'ready_for_feedback'; + component.onUploadSuccess({id: 8, project_id: 1, status: 'ready_for_feedback'}); + component.onUploadComplete(); + + // The handover plays first, so the panel is mid-swap and not yet showing + // the confirmation. + expect(component.flowSwapping).toBe(true); + expect(component.celebration).toBeNull(); + + vi.advanceTimersByTime(600); + expect(component.flowSwapping).toBe(false); + expect(component.celebration?.headline).toBe('Submitted on time. Ready for feedback'); + expect(dialogRef.close).not.toHaveBeenCalled(); + + // Still open well after the sequence itself has finished arriving, so there + // is time to read it rather than catch it. + vi.advanceTimersByTime(2500); + expect(dialogRef.close).not.toHaveBeenCalled(); + + vi.advanceTimersByTime(1500); + expect(dialogRef.close).toHaveBeenCalledWith({value: task}); + vi.useRealTimers(); + }); + + it('lets someone in a hurry leave on the confirmation without a warning', () => { + (component as unknown as {fileUploader: unknown}).fileUploader = { + isUploading: true, + uploadInFlight: true, + hasSelectedFiles: () => true, + }; + const confirm = vi.spyOn(window, 'confirm'); + + // Mid-upload the dialog holds on, files selected and request in flight. + expect(component.canClose()).toBe(false); + + // Once it is confirmed the work is saved, so the backdrop and Escape work. + component.celebration = { + timing: 'on_time', + resubmission: false, + tone: 'success', + particles: false, + headline: 'Submitted on time. Ready for feedback', + detail: '1.1P Hello World', + }; + expect(component.canClose()).toBe(true); + expect(confirm).not.toHaveBeenCalled(); + confirm.mockRestore(); + }); + + it('closes early when the confirmation is dismissed, and only once', () => { + vi.useFakeTimers(); + (task.processTaskStatusChange as unknown as Mock).mockReturnValue({ + timing: 'after_due', + resubmission: false, + tone: 'neutral', + particles: false, + headline: 'Submitted after the due date', + detail: 'T1 A task', + }); + + component.submissionType = 'ready_for_feedback'; + component.onUploadSuccess({id: 8, project_id: 1, status: 'ready_for_feedback'}); + component.onUploadComplete(); + vi.advanceTimersByTime(600); + + component.finishCelebration(); + expect(dialogRef.close).toHaveBeenCalledTimes(1); + + vi.advanceTimersByTime(5000); + expect(dialogRef.close).toHaveBeenCalledTimes(1); + vi.useRealTimers(); + }); + it('restores selection controls after a cancelled slow upload', () => { component.uploadStarted = true; component.uploadSubmitLocked = true; @@ -110,4 +288,90 @@ describe('UploadSubmissionModalComponent', () => { expect(component.uploadSubmitLocked).toBe(false); expect(component.currentStage).toBe('details'); }); + + it('lists the steps and marks the current one', () => { + expect(component.steps.map((step) => step.label)).toEqual(['Upload files', 'Comments']); + expect(component.currentStepIndex).toBe(0); + + component.onReadyChange(true); + component.goToCommentsStage(); + expect(component.currentStepIndex).toBe(1); + }); + + it('explains a disabled forward button and clears once the file is chosen', () => { + expect(component.shouldDisableNext()).toBe(true); + expect(component.continueHint).toBe('Add the required file to continue'); + + component.onReadyChange(true); + expect(component.continueHint).toBeNull(); + }); + + it('carries one panel from sending through to the confirmation', () => { + const uploader = { + uploadProgress: 40, + uploadLanded: false, + uploadingFileLabel: 'report.pdf', + uploadingInfo: {success: null}, + cancelUpload: vi.fn(), + }; + (component as unknown as {fileUploader: unknown}).fileUploader = uploader; + + // Nothing until Submit is pressed. + expect(component.showSubmitFlow).toBe(false); + + component.uploadStarted = true; + expect(component.showSubmitFlow).toBe(true); + expect(component.flowTitle).toBe('Uploading your work'); + expect(component.flowDetail).toBe('report.pdf'); + expect(component.flowProgress).toBe(40); + expect(component.flowLanded).toBe(false); + + // The bytes are away, but the confirmation has not arrived yet. + uploader.uploadLanded = true; + expect(component.flowLanded).toBe(true); + expect(component.flowTitle).toBe('Uploaded'); + + // The same panel becomes the confirmation. + component.celebration = { + timing: 'on_time', + resubmission: false, + tone: 'success', + particles: true, + headline: 'Submitted on time. Ready for feedback', + detail: '1.1P Hello World', + }; + expect(component.showSubmitFlow).toBe(true); + expect(component.flowTitle).toBe('Submitted on time. Ready for feedback'); + expect(component.flowDetail).toBe('1.1P Hello World'); + expect(component.flowProgress).toBe(100); + }); + + it('hands a failure back to the uploader rather than holding the panel', () => { + (component as unknown as {fileUploader: unknown}).fileUploader = { + uploadProgress: 100, + uploadLanded: false, + uploadingFileLabel: 'report.pdf', + uploadingInfo: {success: false}, + }; + component.uploadStarted = true; + + expect(component.uploadFailed).toBe(true); + expect(component.showSubmitFlow).toBe(false); + }); + + it('drops the requirements callout when one drop zone already says the same thing', () => { + expect(component.showUploadRequirements).toBe(false); + + task.definition.uploadRequirements = [ + {key: 'file0', name: 'Report', type: 'document'}, + {key: 'file1', name: 'Code', type: 'code'}, + ] as never; + expect(component.showUploadRequirements).toBe(true); + }); + + it('shows a status icon only for submission types that are task statuses', () => { + expect(component.statusFor('ready_for_feedback')).toBe('ready_for_feedback'); + expect(component.statusFor('reupload_evidence')).toBeNull(); + expect(component.selectedSubmissionTypeLabel).toBe(''); + }); }); diff --git a/src/app/tasks/modals/upload-submission-modal/upload-submission-modal.component.ts b/src/app/tasks/modals/upload-submission-modal/upload-submission-modal.component.ts index 57ba7972f0..a0f163105c 100644 --- a/src/app/tasks/modals/upload-submission-modal/upload-submission-modal.component.ts +++ b/src/app/tasks/modals/upload-submission-modal/upload-submission-modal.component.ts @@ -1,14 +1,38 @@ -import {ChangeDetectionStrategy, Component, Inject, OnInit, ViewChild} from '@angular/core'; +import { + ChangeDetectionStrategy, + Component, + Inject, + OnDestroy, + OnInit, + ViewChild, +} from '@angular/core'; import {MAT_DIALOG_DATA, MatDialogRef} from '@angular/material/dialog'; import {MemberContribution} from 'src/app/api/models/groups/group'; import {Task} from 'src/app/api/models/task'; -import {TaskStatusEnum} from 'src/app/api/models/task-status'; +import {TaskStatus, TaskStatusEnum} from 'src/app/api/models/task-status'; import {ProjectService} from 'src/app/api/services/project.service'; import {TaskService} from 'src/app/api/services/task.service'; +import {prefersReducedMotion} from 'src/app/common/celebrate/reduced-motion'; +import type {SubmissionCelebration} from 'src/app/common/celebrate/submission-timing'; import {FileUploaderComponent} from 'src/app/common/file-uploader/file-uploader.component'; import {AlertService} from 'src/app/common/services/alert.service'; import {EmojiService} from 'src/app/common/services/emoji.service'; import {PrivacyPolicy} from 'src/app/config/privacy-policy/privacy-policy'; +import {summariseUploadRequirement} from './task-upload-requirements/upload-category'; + +/** + * The confirmation finishes arriving at about 1.6s. This leaves roughly two more + * seconds to take it in before the dialog closes itself, and the Done button is + * there for anyone who would rather not wait. + */ +const SUBMISSION_CELEBRATION_HOLD_MS = 3600; + +/** + * The handover: long enough for the bar to draw into the middle (300ms) and the + * circle to answer it (from 240ms). Matches the "collapse" timing on + * /submit-motion, where the six candidates were compared. + */ +const FLOW_SWAP_MS = 480; type UploadStage = 'group' | 'details' | 'comments'; type UploadSubmissionType = TaskStatusEnum | 'reupload_evidence' | 'test_submission'; @@ -18,6 +42,11 @@ interface UploadSubmissionTypeOption { label: string; } +export interface UploadSubmissionStep { + stage: UploadStage; + label: string; +} + interface UploadSubmissionFileSpec { name: string; type: string; @@ -61,7 +90,7 @@ export type UploadSubmissionModalResult = changeDetection: ChangeDetectionStrategy.Eager, standalone: false, }) -export class UploadSubmissionModalComponent implements OnInit { +export class UploadSubmissionModalComponent implements OnInit, OnDestroy { @ViewChild(FileUploaderComponent) private fileUploader?: FileUploaderComponent; public readonly minCommentLength = 25; @@ -89,9 +118,17 @@ export class UploadSubmissionModalComponent implements OnInit { public isUploaderReady = false; public uploadStarted = false; public uploadSubmitLocked = false; + /** Set once the submission has landed. It takes over the dialog until it closes. */ + public celebration: SubmissionCelebration | null = null; + /** The handover: the bar collapses and the circle absorbs it. */ + public flowSwapping = false; private uploadResponse: UploadSubmissionResponse | null = null; + /** Guards the status change against running twice, or not at all. */ + private completionApplied = false; private startUpload?: () => void; + private celebrationTimer: ReturnType | null = null; + private swapTimer: ReturnType | null = null; constructor( @Inject(MAT_DIALOG_DATA) public data: UploadSubmissionModalData, @@ -117,6 +154,16 @@ export class UploadSubmissionModalComponent implements OnInit { return this.fileUploader?.isUploading ?? false; } + /** + * Bytes on the wire, as opposed to a request that has landed and is reporting + * its outcome. `isUploading` covers both, so using it to decide whether the + * dialog may close left it shut after a failed upload: the error panel offers + * Try again and Cancel, but Escape and the backdrop did nothing. + */ + public get uploadInFlight(): boolean { + return this.fileUploader?.uploadInFlight ?? false; + } + public get showGroupSection(): boolean { return this.submissionType === 'ready_for_feedback' && this.task.isGroupTask(); } @@ -167,6 +214,141 @@ export class UploadSubmissionModalComponent implements OnInit { return 'Make a comment...'; } + // ---- The submit panel ---- + // One panel carries the whole thing: the same circle, the same two lines of + // text and the same bar go from sending to sent to confirmed. Nothing is + // unmounted and remounted in between, so there is nothing to read as a new + // screen appearing. + + /** True from the moment Submit is pressed until the dialog closes. */ + public get showSubmitFlow(): boolean { + return this.uploadStarted && !this.uploadFailed; + } + + public get uploadFailed(): boolean { + return this.fileUploader?.uploadingInfo?.success === false; + } + + public get flowProgress(): number { + return this.celebration ? 100 : (this.fileUploader?.uploadProgress ?? 0); + } + + /** The bytes are away. The panel starts becoming the confirmation here. */ + public get flowLanded(): boolean { + return !!this.celebration || this.fileUploader?.uploadLanded === true; + } + + public get flowTitle(): string { + if (this.celebration) { + return this.celebration.headline; + } + return this.flowLanded ? 'Uploaded' : 'Uploading your work'; + } + + public get flowDetail(): string { + return this.celebration?.detail ?? this.fileUploader?.uploadingFileLabel ?? ''; + } + + public cancelUpload(): void { + this.fileUploader?.cancelUpload(); + } + + /** + * The callout above the drop zones only earns its place when it says something + * they do not. A single requirement is already named, typed and stated as + * required by its own zone, so repeating it three ways just adds to the page. + */ + public get showUploadRequirements(): boolean { + const requirements = this.task.definition.uploadRequirements ?? []; + if (requirements.length !== 1) { + return requirements.length > 1; + } + + const summary = summariseUploadRequirement(requirements[0]); + return summary.hasMoreExtensions || summary.maxSizeLabel !== null; + } + + /** The steps this submission goes through, in order. */ + public get steps(): UploadSubmissionStep[] { + const steps: UploadSubmissionStep[] = []; + if (this.showGroupSection) { + steps.push({stage: 'group', label: 'Rate team'}); + } + steps.push({stage: 'details', label: 'Upload files'}); + if (this.showCommentsSection) { + steps.push({ + stage: 'comments', + label: this.submissionType === 'need_help' ? 'Ask for help' : 'Comments', + }); + } + return steps; + } + + public get currentStepIndex(): number { + return Math.max( + 0, + this.steps.findIndex((step) => step.stage === this.currentStage), + ); + } + + /** The task's due or target date, or null when it cannot be worked out. */ + public get dueDate(): Date | null { + try { + const date = this.task.localDueDate?.(); + return date instanceof Date && !Number.isNaN(date.getTime()) ? date : null; + } catch { + return null; + } + } + + public get isPastDue(): boolean { + try { + return this.task.isPastDueDate?.() === true; + } catch { + return false; + } + } + + /** A task status for the icon beside a submission type, when that type is a status. */ + public statusFor(type: UploadSubmissionType): TaskStatusEnum | null { + return TaskStatus.STATUS_KEYS.includes(type as TaskStatusEnum) + ? (type as TaskStatusEnum) + : null; + } + + public get selectedSubmissionTypeLabel(): string { + return ( + this.submissionTypeOptions.find((option) => option.id === this.submissionType)?.label ?? '' + ); + } + + /** Says why the forward button is disabled, or null when it is enabled. */ + public get continueHint(): string | null { + if (this.isGroupStage) { + return this.shouldDisableNext() ? 'Rate a team member to continue' : null; + } + + const forwardDisabled = + this.isDetailsStage && this.showCommentsSection + ? this.shouldDisableNext() + : this.shouldDisableSubmit(); + if (!forwardDisabled) { + return null; + } + + if (!this.isUploaderReady) { + return this.task.definition.uploadRequirements.length > 1 + ? 'Add the required files to continue' + : 'Add the required file to continue'; + } + + if (this.requiresComment && this.comment.trim().length < this.minCommentLength) { + return `Add a comment of at least ${this.minCommentLength} characters`; + } + + return null; + } + public get hasRatedTeamMember(): boolean { return this.team.memberContributions.some((member) => !!member.rating); } @@ -222,7 +404,15 @@ export class UploadSubmissionModalComponent implements OnInit { }; public canClose(): boolean { - if (this.isUploading) { + // Material runs this for programmatic closes as well as Escape and the + // backdrop, so it decides whether the dialog may close itself. Once the + // server has taken the submission there is nothing left to discard, and a + // prompt here would both lie to the student and veto the dialog's own exit: + // answering "Cancel" to "discard your files?" would keep them in it. + if (this.celebration || this.uploadResponse) { + return true; + } + if (this.uploadInFlight) { return false; } if (!this.isDirty) { @@ -255,6 +445,13 @@ export class UploadSubmissionModalComponent implements OnInit { this.currentStage = 'details'; }; + public onUploadFailed = (): void => { + // Only the success path cleared this, so after a failed upload the dialog's + // own Submit stayed locked for good and the uploader's Try again was the + // only way back. + this.uploadSubmitLocked = false; + }; + public onBeforeUpload = (): void => { Object.keys(this.payload).forEach((key) => delete this.payload[key]); @@ -313,23 +510,109 @@ export class UploadSubmissionModalComponent implements OnInit { return; } - const response = this.uploadResponse; - - if (!this.data.isTestSubmission) { - const expectedStatus = - this.submissionType === 'need_help' || this.submissionType === 'ready_for_feedback' - ? this.submissionType - : response.status; + // The dialog the student started this from is still the thing they are + // looking at, so the confirmation belongs here rather than over the page + // they are about to be returned to. + const celebration = this.applyCompletion(true); - this.task.updateFromJson(response, this.taskService.mapping); - this.task.processTaskStatusChange(expectedStatus as TaskStatusEnum, this.alertService, true); + if (celebration) { + this.showCelebration(celebration); + return; } this.dialogRef.close({value: this.task}); }; + /** + * Fold the response into the task. Done can be pressed in the second or so + * between the bytes landing and this callback firing, so it runs at most once + * and either caller may be the one to run it. + * + * Leaving early passes `claimCelebration: false`, which hands the moment to + * the dashboard rather than spending it on a dialog that is already closing. + */ + private applyCompletion(claimCelebration: boolean): SubmissionCelebration | null { + if (this.completionApplied || !this.uploadResponse?.id) { + return null; + } + this.completionApplied = true; + + if (this.data.isTestSubmission) { + return null; + } + + const response = this.uploadResponse; + const expectedStatus = + this.submissionType === 'need_help' || this.submissionType === 'ready_for_feedback' + ? this.submissionType + : response.status; + + this.task.updateFromJson(response, this.taskService.mapping); + return this.task.processTaskStatusChange( + expectedStatus as TaskStatusEnum, + this.alertService, + true, + claimCelebration, + ); + } + + /** Hold long enough for the mark to draw and the words to be read, then leave. */ + private showCelebration(celebration: SubmissionCelebration): void { + if (prefersReducedMotion()) { + this.celebration = celebration; + this.celebrationTimer = setTimeout(() => this.finishCelebration(), 900); + return; + } + + // Play the handover before swapping the words. Without the pause the text is + // replaced between two frames, which reads as a cut however well the box + // around it is transitioning. + this.flowSwapping = true; + this.swapTimer = setTimeout(() => { + this.swapTimer = null; + this.celebration = celebration; + this.flowSwapping = false; + this.celebrationTimer = setTimeout( + () => this.finishCelebration(), + SUBMISSION_CELEBRATION_HOLD_MS, + ); + }, FLOW_SWAP_MS); + } + + /** + * Also the Done button, so nobody has to wait out the hold. Done is on screen + * from the moment the upload lands, which can be before the completion + * callback has run, so apply what it would have applied before leaving. + */ + public finishCelebration(): void { + this.clearFlowTimers(); + this.applyCompletion(false); + this.dialogRef.close({value: this.task}); + } + + private clearFlowTimers(): void { + if (this.celebrationTimer) { + clearTimeout(this.celebrationTimer); + this.celebrationTimer = null; + } + if (this.swapTimer) { + clearTimeout(this.swapTimer); + this.swapTimer = null; + } + } + + public ngOnDestroy(): void { + this.clearFlowTimers(); + // The dialog can go before the uploader's completion callback arrives: + // Escape or the backdrop on the "Uploaded" panel, or Done during the + // handover. The submission is already on the server either way, so record + // it, and leave the celebration unclaimed for the dashboard to show. Does + // nothing when the response never arrived, or has already been applied. + this.applyCompletion(false); + } + public uploadButtonClicked(): void { - if (this.uploadSubmitLocked || this.isUploading) { + if (this.uploadSubmitLocked || this.uploadInFlight) { return; } @@ -362,6 +645,7 @@ export class UploadSubmissionModalComponent implements OnInit { this.uploadStarted = false; this.uploadSubmitLocked = false; this.uploadResponse = null; + this.completionApplied = false; this.currentStage = this.showGroupSection ? 'group' : 'details'; } diff --git a/src/app/tasks/task-comments-viewer/extension-comment/extension-comment.component.html b/src/app/tasks/task-comments-viewer/extension-comment/extension-comment.component.html index 79a7157e8c..8224396ce2 100644 --- a/src/app/tasks/task-comments-viewer/extension-comment/extension-comment.component.html +++ b/src/app/tasks/task-comments-viewer/extension-comment/extension-comment.component.html @@ -4,16 +4,27 @@ [class.ext-card--decided]="comment.assessed" >
- +
- Extension request + {{ + comment.automated ? 'Extended automatically' : 'Extension request' + }} {{ comment.weeksRequested }} {{ comment.weeksRequested === 1 ? 'week' : 'weeks' }}
+ - {{ comment.author?.name }} + @if (comment.automated) { + By {{ externalName | async }} + } @else { + {{ comment.author?.name }} + } @if (comment.createdAt) { · {{ comment.createdAt | date: 'd MMM y' }} } @@ -22,8 +33,10 @@
- Reason -

{{ comment.text }}

+ {{ comment.automated ? 'Why' : 'Reason' }} + +
@if (comment.assessed) { @@ -44,6 +57,7 @@
-

{{ result.error }}

- } -

- Your OnTrack form stays open. After sending in Teams, copy the meeting’s join link back into the - session and save it. -

-

- Teams may ask you to sign in or choose its app or website. If the draft does not open on your - phone, use a desktop browser or copy the details into Teams. -

+
+ @if (result.url) { + Open Teams draft + } @else { + +

{{ result.error }}

+ } +
+
+ +
+

+ Your OnTrack form stays open. After sending in Teams, copy the meeting’s join link back into + the session and save it. +

+

+ Teams may ask you to sign in or choose its app or website. If the draft does not open on + your phone, use a desktop browser or copy the details into Teams. +

+
+
diff --git a/src/app/unit-hub/teams-meeting-composer.component.scss b/src/app/unit-hub/teams-meeting-composer.component.scss index 55ece4f233..120acca547 100644 --- a/src/app/unit-hub/teams-meeting-composer.component.scss +++ b/src/app/unit-hub/teams-meeting-composer.component.scss @@ -1,35 +1,59 @@ +@use './hub-card' as hub; + +@include hub.card-theme; + :host { display: block; min-width: 0; + // every Unit Hub button: the app's filled or outlined style, at a 44px touch height + --mat-button-filled-container-height: 44px; + --mat-button-outlined-container-height: 44px; } +// One card. Inside it, labelled groups separated by spacing, never by rules. +// The host page can set --hub-composer-surface when the composer sits inside another card. .teams-composer { - margin-top: 24px; + @include hub.card(raised); + display: flex; + flex-direction: column; + gap: 16px; + margin-top: 8px; padding: 16px; - border: 1px solid var(--ot-color-divider); - border-radius: var(--ot-radius-md); - background: color-mix(in srgb, var(--ot-color-link) 6%, var(--ot-color-surface)); + background: var(--hub-composer-surface, var(--ot-color-surface)); color: var(--ot-color-text); overflow-wrap: anywhere; } -h3 { - margin: 0 0 0.5rem; - font-size: 1rem; - font-weight: 650; +.composer-heading { + @include hub.icon-heading; } p { - margin: 0.6rem 0; + margin: 0; +} +.composer-intro { + margin-top: -8px; + font-size: 0.9rem; + line-height: 1.5; +} + +.composer-group { + display: flex; + flex-direction: column; + gap: 8px; + min-width: 0; +} +.group-label { + @include hub.label; } -.meeting-subject { +.meeting-subject p:not(.group-label) { line-height: 1.6; } .field { display: grid; gap: 0.35rem; - margin: 1rem 0; + margin: 0 0 8px; font-size: 0.85rem; font-weight: 600; } @@ -59,13 +83,45 @@ small, font-size: 0.85rem; font-weight: 400; } -.repeat-note { +.fallback-note { + margin-top: 6px; +} + +// icon beside the copy, no border: it already sits inside the composer card +.composer-note { + display: flex; + align-items: flex-start; + gap: 10px; + line-height: 1.5; + + > mat-icon { + flex-shrink: 0; + width: 18px; + height: 18px; + margin-top: 1px; + font-size: 18px; + color: var(--ot-color-link); + } +} +.repeat-note p { font-weight: 600; } + +.composer-actions { + display: flex; + flex-wrap: wrap; + align-items: center; + gap: 8px 12px; + + a { + text-decoration: none; + } +} .draft-error { // Pulled a fifth of the way to the text colour, which keeps it above 4.5:1 on the - // tinted box in both themes (the plain error token is 4.2:1 there in light). + // card in both themes (the plain error token is 4.2:1 on a tinted box in light). color: color-mix(in srgb, var(--ot-color-error) 80%, var(--ot-color-text)); + font-size: 0.85rem; } input:focus-visible, textarea:focus-visible, @@ -74,12 +130,17 @@ a:focus-visible { outline-offset: 2px; } -@media (max-width: 480px) { +@media (max-width: 599px) { .teams-composer { - padding: 0.8rem; + padding: 12px; + } + .composer-actions { + flex-direction: column; + align-items: stretch; } - a, - button { + .composer-actions a, + .composer-actions button { width: 100%; + min-height: 44px; } } diff --git a/src/app/unit-hub/teams-meeting-composer.component.ts b/src/app/unit-hub/teams-meeting-composer.component.ts index ad6f0f918b..16ef1f33c7 100644 --- a/src/app/unit-hub/teams-meeting-composer.component.ts +++ b/src/app/unit-hub/teams-meeting-composer.component.ts @@ -2,12 +2,13 @@ import {CommonModule} from '@angular/common'; import {ChangeDetectionStrategy, Component, Input, OnChanges} from '@angular/core'; import {FormControl, ReactiveFormsModule} from '@angular/forms'; import {MatButtonModule} from '@angular/material/button'; +import {MatIconModule} from '@angular/material/icon'; import {TeamsMeetingDraft, teamsMeetingContent, teamsMeetingDraftUrl} from './teams-meeting-draft'; @Component({ selector: 'f-teams-meeting-composer', standalone: true, - imports: [CommonModule, ReactiveFormsModule, MatButtonModule], + imports: [CommonModule, ReactiveFormsModule, MatButtonModule, MatIconModule], templateUrl: './teams-meeting-composer.component.html', styleUrl: './teams-meeting-composer.component.scss', changeDetection: ChangeDetectionStrategy.Eager, diff --git a/src/app/unit-hub/unit-hub-details.component.html b/src/app/unit-hub/unit-hub-details.component.html index 0f44f8bfd1..3a915f492e 100644 --- a/src/app/unit-hub/unit-hub-details.component.html +++ b/src/app/unit-hub/unit-hub-details.component.html @@ -1,8 +1,18 @@ @if (content; as detail) { -
+

{{ detail.unitCode }} · {{ detail.unitName }}

-

{{ title }}

+

+ @if (cancelled) { + + } + {{ title }} +

+ @if (cancelled) { +

+ Cancelled: this session will not run +

+ }
} @@ -173,6 +210,19 @@

Choose how to add this session

} }

{{ status }}

+ + + @@ -193,15 +243,19 @@

Choose how to add this session

rel="noopener noreferrer" target="_blank" [href]="safeUrl(detail.googleCalendarUrl)" + >{{ detail.demo ? 'Preview DEMO in Google Calendar' : 'Add to Google Calendar' }} } @if (detail.download) { - } } - + } diff --git a/src/app/unit-hub/unit-hub-details.component.scss b/src/app/unit-hub/unit-hub-details.component.scss index cde87b88c2..981f0ec573 100644 --- a/src/app/unit-hub/unit-hub-details.component.scss +++ b/src/app/unit-hub/unit-hub-details.component.scss @@ -1,9 +1,16 @@ +@use './hub-card' as hub; + +@include hub.card-theme; + :host { display: flex; flex-direction: column; min-height: 0; max-height: 90dvh; color: var(--ot-color-text); + // every Unit Hub button: the app's filled or outlined style, at a 44px touch height + --mat-button-filled-container-height: 44px; + --mat-button-outlined-container-height: 44px; } .details-header { display: flex; @@ -40,7 +47,11 @@ h2[mat-dialog-title] { h2[mat-dialog-title]::before { display: none; } +// grouping comes from cards and one vertical rhythm, not rules between sections .details-content { + display: flex; + flex-direction: column; + gap: 16px; min-height: 0; flex: 1; max-height: none; @@ -48,25 +59,23 @@ h2[mat-dialog-title]::before { color: inherit; overflow-wrap: anywhere; } +.details-content > * { + margin: 0; +} +// the dialog is a raised surface, so every card here takes the surface token .detail-facts { + @include hub.card(raised); display: grid; - grid-template-columns: 1fr 1fr; + grid-template-columns: minmax(0, 1fr) minmax(0, 1fr); gap: 16px 24px; - margin: 16px 0 24px; + margin-top: 10px; padding: 16px; - border: 1px solid var(--ot-color-divider); - border-radius: var(--ot-radius-md); - background: var(--ot-color-surface-raised); } .detail-facts > div { min-width: 0; } dt { - color: var(--ot-color-text-muted); - font-size: 0.72rem; - font-weight: 600; - letter-spacing: 0.04em; - text-transform: uppercase; + @include hub.label; margin-bottom: 4px; } dd { @@ -80,85 +89,86 @@ small { font-size: 0.76rem; margin-top: 3px; } +// the body gets its own quiet card on the same inset as the facts card .full-content { + @include hub.card(raised); + padding: 16px; line-height: 1.7; - border-top: 1px solid var(--ot-color-divider); - padding-top: 16px; } -.full-content p { - white-space: pre-wrap; - margin: 0; +.content-label { + @include hub.label; + margin-bottom: 8px; } -.detail-notice, -.calendar-guidance { - background: var(--ot-color-surface-raised); - padding: 16px; - border: 1px solid var(--ot-color-divider); - border-radius: var(--ot-radius-md); - margin: 6px 0 18px; - font-size: 0.84rem; - line-height: 1.6; +// markdown body: a comfortable measure and rhythm for long announcements +.markdown-body { + @include hub.markdown; +} +.markdown-body.is-clamped { + max-height: 40vh; + overflow: hidden; + // fades the last lines out so it reads as continuing; a mask, so no colour is involved + -webkit-mask-image: linear-gradient(to bottom, black 70%, transparent); + mask-image: linear-gradient(to bottom, black 70%, transparent); +} +.body-toggle { + margin-top: 12px; } .detail-notice { - display: flex; - align-items: flex-start; - gap: 12px; + @include hub.callout(raised); + p { + margin: 0; + } } -.detail-notice mat-icon { - flex-shrink: 0; +// a cancelled session: struck title in the header, the banner under it +.details-header.is-cancelled { + h2[mat-dialog-title] { + text-decoration: line-through; + text-decoration-thickness: 1px; + } } -.detail-notice p { - margin: 0; +.cancelled-banner { + @include hub.cancelled-banner; + margin-top: 10px; +} +.visually-hidden { + position: absolute; + width: 1px; + height: 1px; + margin: -1px; + padding: 0; + overflow: hidden; + clip: rect(0, 0, 0, 0); + white-space: nowrap; + border: 0; } .calendar-guidance { - margin: 24px 0 0; + @include hub.card(raised); + padding: 16px; + font-size: 0.84rem; + line-height: 1.6; } .calendar-guidance h3 { - font-size: 1rem; - font-weight: 650; - margin: 0 0 8px; + @include hub.icon-heading; + margin-bottom: 8px; } .calendar-guidance p { - margin: 0 0 8px; -} -.cancelled-notice { - color: var(--ot-color-text); - border: 1px solid var(--ot-color-error); - border-left-width: 4px; - padding: 12px 16px; - border-radius: var(--ot-radius-sm); + margin: 0 0 12px; } .detail-source { - display: inline-flex; - align-items: center; - gap: 6px; - margin-top: 22px; - color: var(--ot-color-link); - text-underline-offset: 3px; -} -.detail-source mat-icon { - font-size: 18px; - width: 18px; - height: 18px; + align-self: flex-start; + text-decoration: none; } .details-actions { - display: flex; - flex-wrap: wrap; + @include hub.action-row; flex-shrink: 0; justify-content: flex-end; - gap: 8px; - border-top: 1px solid var(--ot-color-divider); padding: 14px 24px; } -.details-actions a { - text-decoration: none; -} .detail-status:empty { display: none; } .detail-status { font-size: 0.84rem; - margin-top: 14px; } a:focus-visible, button:focus-visible { @@ -176,18 +186,13 @@ button:focus-visible { padding: 12px 18px; } .detail-facts { - grid-template-columns: 1fr; + grid-template-columns: minmax(0, 1fr); gap: 14px; } h2[mat-dialog-title] { font-size: 1.22rem; } - .details-actions a, - .details-actions button { - min-height: 44px; + .detail-source { + align-self: stretch; } } - -.full-content p { - font-size: 0.95rem; -} diff --git a/src/app/unit-hub/unit-hub-details.component.spec.ts b/src/app/unit-hub/unit-hub-details.component.spec.ts index 5c639f0dad..a654836a21 100644 --- a/src/app/unit-hub/unit-hub-details.component.spec.ts +++ b/src/app/unit-hub/unit-hub-details.component.spec.ts @@ -121,4 +121,29 @@ describe('Unit Hub full details', () => { expect(calendarSettings).toHaveBeenCalledOnce(); expect(close).toHaveBeenCalledOnce(); }); + it('renders a markdown body and clamps a long one behind an expand toggle', async () => { + const announcement = { + ...unitHubDemo(new Date('2026-09-14T06:00:00Z')).announcements[0], + body: '**Bold** start\n\n[Guide](https://example.test/guide)', + }; + const {component, fixture, element} = await render({announcement}); + const body = element.querySelector('#unit-hub-detail-body') as HTMLElement; + expect(body.querySelector('strong')?.textContent).toBe('Bold'); + expect(body.querySelector('a')?.getAttribute('rel')).toBe('noopener noreferrer'); + expect(element.querySelector('.body-toggle')).toBeNull(); + + Object.defineProperty(body, 'scrollHeight', {configurable: true, value: 5000}); + component.measureBody(); + fixture.detectChanges(); + const toggle = element.querySelector('.body-toggle') as HTMLButtonElement; + expect(toggle.textContent).toContain('Show full announcement'); + expect(toggle.getAttribute('aria-expanded')).toBe('false'); + expect(toggle.getAttribute('aria-controls')).toBe('unit-hub-detail-body'); + expect(body.classList).toContain('is-clamped'); + toggle.click(); + fixture.detectChanges(); + expect(toggle.getAttribute('aria-expanded')).toBe('true'); + expect(body.classList).not.toContain('is-clamped'); + expect(toggle.textContent).toContain('Show less'); + }); }); diff --git a/src/app/unit-hub/unit-hub-details.component.ts b/src/app/unit-hub/unit-hub-details.component.ts index e07c277051..f84bc4f339 100644 --- a/src/app/unit-hub/unit-hub-details.component.ts +++ b/src/app/unit-hub/unit-hub-details.component.ts @@ -1,8 +1,19 @@ import {CommonModule} from '@angular/common'; -import {ChangeDetectionStrategy, Component, Inject} from '@angular/core'; +import { + ChangeDetectionStrategy, + ChangeDetectorRef, + Component, + ElementRef, + Inject, + Injector, + ViewChild, + afterNextRender, + inject, +} from '@angular/core'; import {MatButtonModule} from '@angular/material/button'; import {MAT_DIALOG_DATA, MatDialogModule, MatDialogRef} from '@angular/material/dialog'; import {MatIconModule} from '@angular/material/icon'; +import {HubMarkdownPipe} from './hub-markdown'; import {TeamsMeetingComposerComponent} from './teams-meeting-composer.component'; import {safeHttpsUrl} from './unit-hub-calendar'; import {LearningSession, SESSION_KINDS, UnitAnnouncement} from './unit-hub.models'; @@ -28,6 +39,7 @@ export interface UnitHubDetailsData { MatButtonModule, MatDialogModule, MatIconModule, + HubMarkdownPipe, TeamsMeetingComposerComponent, ], templateUrl: './unit-hub-details.component.html', @@ -39,18 +51,44 @@ export class UnitHubDetailsComponent { readonly safeUrl = safeHttpsUrl; content: UnitHubDetailsData | null; status = ''; + /** A body taller than this share of the viewport starts clamped behind a toggle. */ + readonly clampShare = 0.4; + bodyCollapsible = false; + bodyExpanded = false; + @ViewChild('bodyText') private bodyText?: ElementRef; + private readonly changes = inject(ChangeDetectorRef); constructor( @Inject(MAT_DIALOG_DATA) data: UnitHubDetailsData, private dialogRef: MatDialogRef, ) { this.content = data; + afterNextRender(() => this.measureBody(), {injector: inject(Injector)}); + } + + /** Decides whether the body is long enough to clamp. Its full height is kept while clamped. */ + measureBody(): void { + const body = this.bodyText?.nativeElement; + const limit = window.innerHeight * this.clampShare; + const collapsible = !!body && body.scrollHeight > limit; + if (collapsible !== this.bodyCollapsible) { + this.bodyCollapsible = collapsible; + this.changes.detectChanges(); + } + } + + toggleBody(): void { + this.bodyExpanded = !this.bodyExpanded; } get title(): string { return this.content?.announcement?.title ?? this.content?.session?.title ?? ''; } + get cancelled(): boolean { + return this.content?.session?.cancelled === true; + } + get sourceLabel(): string { const announcement = this.content?.announcement; return announcement?.managed_externally || announcement?.source_provider === 'microsoft_teams' diff --git a/src/app/unit-hub/unit-hub.component.html b/src/app/unit-hub/unit-hub.component.html index 77727f088a..b5928a1572 100644 --- a/src/app/unit-hub/unit-hub.component.html +++ b/src/app/unit-hub/unit-hub.component.html @@ -10,45 +10,43 @@

Unit Hub

@if (showFeed) { - + + }
@if (manageableUnits.length) { } + Get notified } + + Get notified +
@@ -99,7 +113,7 @@

Unit Hub

@if (demo.enabled) { } +

{{ status }}

@if (loading) { @@ -143,7 +158,9 @@

Unit Hub

Updates are unavailable

{{ loadError }}

- + } @else if (managing) {
@@ -156,7 +173,7 @@

Manage unit updates

- -

- @if (managedTeamsConfigured) { - Automatic Teams announcements are configured for this unit. Posts appear after a - successful update. Change imported posts in Teams; they cannot be edited here. - } @else { - Publish updates here or link to their original Teams post. Your university can connect - this unit's Teams announcements through administrator setup. Microsoft sign-in alone does - not turn that connection on. - } - Session schedules are maintained here. -

- @if (staffError) { -
+ +

+ @if (managedTeamsConfigured) { + Automatic Teams announcements are configured for this unit. Posts appear after a + successful update. Change imported posts in Teams; they cannot be edited here. + } @else { + Publish updates here or link to their original Teams post. Your university can connect + this unit's Teams announcements through administrator setup. Microsoft sign-in alone + does not turn that connection on. + } + Session schedules are maintained here.

+
+ @if (staffError) { + } @if (staffLoading) {

Loading drafts and published content…

} @if (!staffLoading && canManageSelected) { -
+
campaignNew announcement
@if (editor) {
-

{{ editingId ? 'Edit' : 'New' }} {{ editor }}

+

+ {{ editingId ? 'Edit' : 'New' }} + {{ editor }} +

@if (editor === 'announcement') {
- - - - - - +

+ Details +

+ + +
+

+ Supports simple formatting: **bold**, *italic*, lists, links +

+
+ + +
+
+ @if (announcementPreview) { +
+ } +
+
+ + +
+
-

- Leave Publish unchecked to save a draft that students cannot see. -

+

+ Visibility +

+ + + +

+ Leave Publish unchecked to save a draft that students cannot see. +

+
} @else {
- -
+
+

+ Details +

+ -
+
+

+ When +

+
+ + + + + @if (sessionForm.controls.recurrence.value === 'weekly') { + + } +
+

+ Enter both times in the named time zone. Weekly sessions keep this local time + when daylight saving changes. +

+
+
+

+ Where +

+ - + -
-

- Enter both times in the named time zone. Weekly sessions keep this local time - when daylight saving changes. -

-
- +

+ Visibility +

+ - @if (sessionForm.controls.recurrence.value === 'weekly') { - + @if (editingId) { + } +

+ Leave Publish unchecked for a draft. Editing a repeating session changes the + whole series. +

- - - - - - @if (editingId) { - - } -

- Leave Publish unchecked for a draft. Editing a repeating session changes the - whole series. -

@@ -374,13 +508,22 @@

{{ editingId ? 'Edit' : 'New' }} {{ editor }}

} @if (formError) { - + } -
+
@@ -388,7 +531,8 @@

{{ editingId ? 'Edit' : 'New' }} {{ editor }}

} @if (deleteCandidate) {