From b142f8b9abe062ce07b0037dfc5f1be02541c148 Mon Sep 17 00:00:00 2001 From: trtshen Date: Tue, 12 May 2026 11:53:32 +0800 Subject: [PATCH] refactor assessment component and improve pagination logic - for team360 especially - update pagination styles for better alignment - track visited pages in assessment component - enhance team 360 assessment logic for page tracking - adjust bottom action bar layout for custom content - simplify task navigation in activity desktop page --- docs/assessment-flow.md | 99 ++-- .../assessment-pagination-feature-toggle.md | 41 +- ...tion-visited-indicator-and-submit-guard.md | 226 ++++++++ .../assessment/assessment.component.html | 36 +- .../assessment/assessment.component.scss | 45 +- .../assessment/assessment.component.spec.ts | 507 +++++++++++++++++- .../assessment/assessment.component.ts | 132 ++++- .../bottom-action-bar.component.html | 2 +- .../bottom-action-bar.component.scss | 7 +- .../activity-desktop/activity-desktop.page.ts | 4 +- 10 files changed, 975 insertions(+), 124 deletions(-) create mode 100644 docs/fixes/pagination-visited-indicator-and-submit-guard.md diff --git a/docs/assessment-flow.md b/docs/assessment-flow.md index 6a267aeaf..a74079832 100644 --- a/docs/assessment-flow.md +++ b/docs/assessment-flow.md @@ -1,3 +1,11 @@ +--- +status: stable +authority: canonical +scope: frontend +last_reviewed: 2026-05-01 +supersedes: none +--- + # Assessment Flow Documentation ## Overview @@ -431,13 +439,27 @@ setSubmissionDisabled() { ### Core Properties ```typescript -pageSize = 8; // Maximum questions per page -pageIndex: number = 0; // Current page (0-based) -pagesGroups: any[] = []; // Pages containing question groups -pageRequiredCompletion: boolean[] = []; // Completion status per page -readonly manyPages = 6; // Minimum pages for scrollable pagination +pageSize = 10; // maximum questions per page +pageIndex: number = 0; // current page (0-based) +pagesGroups: any[] = []; // pages containing question groups +pageRequiredCompletion: boolean[] = []; // completion status per page (required questions answered?) +pageVisited: boolean[] = []; // whether the user has navigated to each page +readonly manyPages = 10; // minimum pages to show scrollable pagination ``` +#### page indicator state model + +| `pageVisited[i]` | `pageRequiredCompletion[i]` | visual state | +|---|---|---| +| `false` | any | plain number (neutral — not yet visited) | +| `true` | `true` | green checkmark (visited + required complete) | +| `true` | `false` | plain number (neutral — visited but incomplete; no red) | + +- on first load, only page 0 is marked visited. +- pages are marked visited when the user navigates to them via `prevPage()`, `nextPage()`, or `goToPage()`. +- `pageVisited` is reset to `[]` in `ngOnChanges` whenever `assessment`, `submission`, or `review` changes. +- in read-only mode all pages are marked visited (all indicators show green). + ### Page Generation Logic ```typescript splitGroupsByQuestionCount() { @@ -450,14 +472,14 @@ splitGroupsByQuestionCount() { ### Navigation Methods ```typescript -prevPage() // Go to previous page with boundary check -nextPage() // Go to next page with boundary check -goToPage(i: number) // Jump to specific page with validation +prevPage() // go to previous page; marks destination as visited +nextPage() // go to next page; marks destination as visited +goToPage(i: number) // jump to specific page; marks target as visited ``` ### Completion Tracking -The completion tracking system has been improved to handle proper initialization timing and avoid the "incompleted" class showing incorrectly on first load. +completion tracking is gated on `pageVisited` so optional-only pages never show false-positive red indicators. ```typescript ngOnChanges(changes: SimpleChanges): void { @@ -468,9 +490,8 @@ ngOnChanges(changes: SimpleChanges): void { this._initialise(); if (changes.assessment || changes.submission || changes.review) { - // reset button state when assessment changes - this.btnDisabled$.next(false); this.pageRequiredCompletion = []; + this.pageVisited = []; // reset visited state on new assessment/submission this._handleSubmissionData(); this._populateQuestionsForm(); @@ -478,34 +499,28 @@ ngOnChanges(changes: SimpleChanges): void { this._prefillForm(); } - // split by question count every time assessment changes - only if pagination is enabled if (this.isPaginationEnabled) { this.pagesGroups = this.splitGroupsByQuestionCount(); this.pageIndex = 0; - // initialize page completion after form is fully set up - // use delay to ensure form values are populated setTimeout(() => { this.initializePageCompletion(); }, 200); } else { - // Reset pagination data when disabled this.pagesGroups = []; this.pageIndex = 0; } - // scroll to the active page into view after rendering setTimeout(() => this.scrollActivePageIntoView(), 250); } initializePageCompletion() { if (!this.isPaginationEnabled) return; - // Only track completion status when user can actually edit the form - // In read-only mode (viewing feedback or completed submissions), completion tracking is not relevant if (!this.doAssessment && !this.isPendingReview) { - // Set all pages as completed for read-only mode to avoid showing incomplete indicators + // read-only mode: mark everything visited and complete this.pageRequiredCompletion = new Array(this.pageCount).fill(true); + this.pageVisited = new Array(this.pageCount).fill(true); this.cdr.detectChanges(); setTimeout(() => this.scrollActivePageIntoView(), 100); return; @@ -513,32 +528,26 @@ initializePageCompletion() { this.pageRequiredCompletion = new Array(this.pageCount).fill(true); + // only initialise visited array on first call; preserve state on re-runs (e.g. form value changes) + if (!this.pageVisited.length) { + this.pageVisited = new Array(this.pageCount).fill(false); + this.pageVisited[0] = true; // page 0 is always visited on load + } + this.pages.forEach((page, index) => { const pageQuestions = this.getAllQuestionsForPage(index); this.pageRequiredCompletion[index] = this.areAllRequiredQuestionsAnswered(pageQuestions); }); - // trigger change detection to update the view this.cdr.detectChanges(); - - // Update the scroll position when page completion status changes setTimeout(() => this.scrollActivePageIntoView(), 100); } ``` -**Key Improvements for First Load Issue:** -1. **Timing Fix**: `initializePageCompletion()` is now called in `ngOnChanges()` after pagination setup with a 200ms delay -2. **Change Detection**: Added `this.cdr.detectChanges()` to ensure the view updates when completion status changes -3. **Form Population Order**: Form values are populated via `_prefillForm()` before completion tracking runs -4. **Race Condition Prevention**: Delayed form valueChanges subscription to avoid interference during initialization -5. **Read-Only Mode Handling**: Completion tracking is disabled when users are viewing feedback or completed submissions (`!doAssessment && !isPendingReview`), showing all pages as completed instead - -## Pagination Issue Fixes - -### Problem: "Incompleted" Class on First Load +### Problem: Stale Completion State on First Load **Issue Description:** -Page indicators showed up with the "incompleted" class on first load of assessments, even when questions were already answered. This occurred due to a timing mismatch between form population and completion tracking initialization. +Page indicators could show an incorrect state on first load of assessments, even when questions were already answered. This occurred due to a timing mismatch between form population and completion tracking initialization. **Root Cause:** The `initializePageCompletion()` method was being called before form values were fully populated, causing `areAllRequiredQuestionsAnswered()` to return false for completed questions. @@ -585,6 +594,28 @@ The `initializePageCompletion()` method was being called before form values were **Result:** Page indicators now correctly show completion status on first load, with proper visual feedback for answered and unanswered required questions. +### Unvisited-pages confirmation guard (Team 360 early-submit prevention) + +**context:** in Team 360 assessments, the first N question groups (pages) contain required questions for mandatory team members; additional pages contain optional questions for extra members. once the required pages are answered the form becomes valid and the submit button enables — but the learner may not have visited the optional pages. clicking submit at that point would silently skip feedback for the remaining team members. + +**solution (`assessment.component.ts:continueToNextTask()`):** + +``` +continueToNextTask() is now async +↓ +on submit action, check: isPaginationEnabled && pageCount > 1 && any pageVisited[i] === false +↓ +if unvisited pages exist → present AlertController dialog + "Review pages" → goToPage(firstUnvisited); cancel submission + "Submit anyway" → proceed to _doSubmit() +↓ +if no unvisited pages → proceed directly to _doSubmit() +``` + +relevant source: `projects/v3/src/app/components/assessment/assessment.component.ts` — `continueToNextTask()`, `_doSubmit()`, `_confirmSubmitWithUnvisitedPages()` + +**the guard is generic** — it fires for any multi-page assessment where the learner has not visited every page, not just Team 360. zero friction when all pages have been visited. + ## Error Handling ### Validation Errors @@ -753,7 +784,7 @@ Key strengths: - Robust error handling and network resilience - Performance optimizations for large assessments - Comprehensive pagination system for long assessments -- **Improved initialization timing** to prevent incorrect "incompleted" status on first load +- **Improved initialization timing** to prevent incorrect completion status on first load - **Proper change detection management** with OnPush strategy - **Race condition prevention** through strategic delays and sequencing diff --git a/docs/features/assessment-pagination-feature-toggle.md b/docs/features/assessment-pagination-feature-toggle.md index c475c43bc..aa7905fda 100644 --- a/docs/features/assessment-pagination-feature-toggle.md +++ b/docs/features/assessment-pagination-feature-toggle.md @@ -1,3 +1,11 @@ +--- +status: stable +authority: canonical +scope: frontend +last_reviewed: 2026-05-05 +supersedes: none +--- + # Assessment Pagination Feature Toggle This document explains how to enable/disable the assessment pagination feature using environment flags. @@ -28,10 +36,24 @@ export const environment = { ## Behavior ### When `assessmentPagination: true` (Default) -- Assessment questions are split across multiple pages (8 questions per page by default) -- Pagination controls (Previous/Next buttons and page indicators) are visible -- Users can navigate between pages using buttons or clicking page indicators -- Page completion indicators show which pages have unanswered required questions +- assessment questions are split across multiple pages (10 questions per page) +- pagination controls (Prev/Next buttons and page indicators) are visible in the bottom action bar +- page indicator states depend on the current mode: + + **edit mode** (`doAssessment = true` or `isPendingReview = true`): + + | visited | required complete | indicator | + |---|---|---| + | no | any | plain number (neutral) | + | yes | no | plain number (neutral) | + | yes | yes | green checkmark | + + **read-only mode** (feedback viewing, completed submission, locked team assessment): + - all indicators show plain numbers only — completion state is irrelevant when nothing is editable + +- when the user clicks Submit and unvisited pages exist, they are submitted directly — no confirmation dialog is shown +- navigating to a new page automatically scrolls the view back to the top of the form (handles both mobile `ion-content` and desktop split-pane `ion-col` scroll containers) +- the submit button is centered directly below the pagination row in the bottom action bar ### When `assessmentPagination: false` - All assessment questions are displayed on a single page @@ -41,12 +63,13 @@ export const environment = { ## Technical Implementation -The feature toggle affects: +the feature toggle affects: -1. **Template Rendering**: Pagination UI is conditionally rendered based on `isPaginationEnabled` -2. **Question Display**: Questions are either paginated or shown all at once via `pagedGroups` getter -3. **Navigation Methods**: Pagination methods (`prevPage`, `nextPage`, etc.) are safe-guarded -4. **Page Completion**: Completion tracking is only active when pagination is enabled +1. **template rendering** — pagination UI is conditionally rendered based on `isPaginationEnabled` +2. **question display** — questions are either paginated or shown all at once via `pagedGroups` getter +3. **navigation methods** — `prevPage()`, `nextPage()`, `goToPage()` are no-ops when pagination is disabled; each marks the destination page in `pageVisited[]` +4. **page completion** — `pageRequiredCompletion[]` tracks whether all required questions on each page are answered; indicators are gated on `pageVisited[]` so unvisited pages always show neutral +5. **submit guard** — `continueToNextTask()` checks `pageVisited[]` before submitting; shows a confirmation alert when any page is unvisited ## Usage Examples diff --git a/docs/fixes/pagination-visited-indicator-and-submit-guard.md b/docs/fixes/pagination-visited-indicator-and-submit-guard.md new file mode 100644 index 000000000..4aef6a832 --- /dev/null +++ b/docs/fixes/pagination-visited-indicator-and-submit-guard.md @@ -0,0 +1,226 @@ +--- +status: stable +author: canonical +scope: frontend +last_reviewed: 2026-05-05 +supersedes: none +--- + +> **2026-05-05 addendum** — follow-up refinements applied after initial merge; see [§ follow-up changes](#follow-up-changes-2026-05-05) below. + +# pagination: visited-indicator state model + unvisited-pages submit guard + +## problems addressed + +### 1 — optional-only pages showed misleading red indicators on load + +in team 360 assessments each question group (page) typically contains a `team-member-selector` question plus follow-up rating/text questions. the first N pages are required; additional pages are optional (extra team members). + +`areAllRequiredQuestionsAnswered()` returns `true` vacuously when a page contains no required questions, so optional pages were always marked complete and showed a green checkmark immediately. conversely, required pages loaded red before the user had a chance to fill them in, causing confusion. + +the red indicator was removed entirely. the root cause was addressed by gating all indicators on whether the user has actually visited a page. + +### 2 — learner could accidentally submit before visiting optional pages + +once required questions on the first N pages were answered, `questionsForm.valid` became `true` and the submit button enabled. a learner could submit without ever seeing the optional pages, silently skipping feedback for additional team members. + +## solution + +### pageVisited state model + +**`projects/v3/src/app/components/assessment/assessment.component.ts`** + +new property added alongside `pageRequiredCompletion`: + +```typescript +pageVisited: boolean[] = []; // tracks which pages the user has navigated to +``` + +| `pageVisited[i]` | `pageRequiredCompletion[i]` | indicator shown | +|---|---|---| +| `false` | any | plain number (neutral) | +| `true` | `true` | green checkmark | +| `true` | `false` | plain number (neutral; required still tracked for submit guard) | + +initialisation rules (inside `initializePageCompletion()`): +- called once per assessment load; only creates the array if it is empty +- page 0 is pre-marked visited (always the landing page) +- in read-only mode all pages are marked visited (all show green) +- `pageVisited` is reset to `[]` in `ngOnChanges` when `assessment`, `submission`, or `review` changes + +navigation methods each mark the destination: +```typescript +prevPage() → this.pageVisited[this.pageIndex] = true +nextPage() → this.pageVisited[this.pageIndex] = true +goToPage(i) → this.pageVisited[i] = true +``` + +template changes (`assessment.component.html`) — both pagination blocks updated: +```html + +[class.completed]="pageVisited[page] && pageRequiredCompletion[page]" + + +``` + +no scss changes needed; the base `.page-indicator` style (neutral grey) already provides the unvisited/incomplete appearance. + +### unvisited-pages submit guard + +> **Removed 2026-05-06** — the guard and its confirmation popup were removed. `continueToNextTask()` is now synchronous and calls `_doSubmit()` directly without checking `pageVisited`. `AlertController` was also removed from the component's constructor and import. See [§ follow-up changes](#follow-up-changes-2026-05-06) for detail. +```typescript +private _doSubmit() { + this.submitting = true; + this.btnDisabled$.next(true); + this.submitActions.next({ autoSave: false, goBack: false }); +} +``` + +`AlertController` was added to the constructor injection alongside the existing `ModalController`. + +the guard is generic — it fires for any multi-page assessment with unvisited pages, not only team 360. when all pages have been visited there is zero added friction. + +## files changed + +| file | change | +|---|---| +| `projects/v3/src/app/components/assessment/assessment.component.ts` | `pageVisited` property; `prevPage`/`nextPage`/`goToPage` mark visited; `initializePageCompletion` initialises + preserves visited; `AlertController` import + injection; `continueToNextTask` async with guard; `_doSubmit()`; `_confirmSubmitWithUnvisitedPages()` | +| `projects/v3/src/app/components/assessment/assessment.component.html` | both pagination blocks: `[class.completed]` gated on `pageVisited`; green checkmark `*ngIf` only; red icon removed | +| `projects/v3/src/app/components/assessment/assessment.component.scss` | removed `.page-indicator.incompleted`, `.page-indicator.incompleted .page-number`, `.page-indicator.incompleted .progress-ring-fill`, `.completion-icon:not(.completed)` | +| `projects/v3/src/app/components/assessment/assessment.component.spec.ts` | `AlertController` spy + provider; all `continueToNextTask` call sites made `async`; four new guard tests; two new `initializePageCompletion` tests; two new `prevPage`/`nextPage` visited tests; one new `goToPage` visited test | + +## tests added + +``` +continueToNextTask() unvisited-pages guard + ✓ submits directly when all pages are visited + ✓ shows alert and submits when user confirms "Submit anyway" + ✓ cancels and navigates when user clicks "Review pages" + ✓ does not show alert when pagination is disabled + +initializePageCompletion() + ✓ marks all pages visited in read-only mode + ✓ marks only page 0 visited on first call in edit mode + ✓ preserves existing pageVisited state across re-runs + +prevPage() / nextPage() + ✓ marks destination page as visited + +goToPage() + ✓ marks target page as visited +``` + +## rationale for removing the red indicator + +the non-tech team requested removing red entirely. this was accepted because: +- the visited gate already prevents false-positive red on optional pages +- showing red on visited + required-incomplete pages adds friction before the user has finished filling in the page; it is confusing rather than helpful +- the submit guard (`_confirmSubmitWithUnvisitedPages`) catches the case where a learner tries to submit early, making the red indicator redundant as a warning mechanism +- `pageRequiredCompletion` is still tracked internally and still drives the submit guard — only the visual representation changed + +## follow-up changes (2026-05-06) + +### 4 — unvisited-pages submit guard removed + +**decision:** the confirmation popup ("You have not visited page(s) X. Submit anyway?") was removed entirely. `continueToNextTask()` reverted to synchronous and calls `_doSubmit()` directly. `AlertController` was removed from imports and the constructor. + +**rationale:** the submit button is now visually positioned below the pagination row, centered, giving the user a natural visual cue to review each page before submitting. the interrupt dialog added friction without adding value. + +**files changed:** + +| file | change | +|---|---| +| `projects/v3/src/app/components/assessment/assessment.component.ts` | `continueToNextTask()` made synchronous; unvisited-pages guard removed; `_confirmSubmitWithUnvisitedPages()` deleted; `AlertController` removed from import + constructor | +| `projects/v3/src/app/components/assessment/assessment.component.spec.ts` | `AlertController` import, provider, spy, and full `continueToNextTask() unvisited-pages guard` describe block removed | + +### 5 — submit button moved below pagination, centered + +**decision:** the submit button is now stacked below the pagination row in the bottom action bar, centered, instead of sitting to the right of the indicators. + +**implementation:** added `.stacked` modifier class to `.action-container` in `bottom-action-bar.component.html` (applied when `hasCustomContent` is true). `.action-container.stacked` uses `flex-direction: column; align-items: center`. `.button-container.with-custom-content` `justify-content` changed from `flex-end` to `center`. + +**files changed:** + +| file | change | +|---|---| +| `projects/v3/src/app/components/bottom-action-bar/bottom-action-bar.component.html` | `[class.stacked]="hasCustomContent"` added to `.action-container` | +| `projects/v3/src/app/components/bottom-action-bar/bottom-action-bar.component.scss` | `.action-container.stacked` — column layout, centered; `.button-container.with-custom-content` `justify-content: center` | + +--- + +## follow-up changes (2026-05-05) + +### 1 — read-only mode: indicators hidden entirely + +**problem:** in read-only mode (feedback-viewing, completed submissions, locked team assessments) `initializePageCompletion()` marks all pages visited with all completion flags `true`, which caused every page indicator to show a green checkmark. this is meaningless — the user is not filling in anything — and clutters the UI. + +**decision:** completion indicators (`.completed` css class + `checkmark-circle` icon) are now gated on `(doAssessment || isPendingReview)` in addition to the existing visited/completion flags. in every read-only scenario both flags are `false`, so indicators are suppressed. + +state table (updated): + +| `doAssessment \|\| isPendingReview` | `pageVisited[i]` | `pageRequiredCompletion[i]` | indicator shown | +|---|---|---|---| +| `false` (read-only) | any | any | plain number only | +| `true` | `false` | any | plain number only | +| `true` | `true` | `true` | green checkmark | +| `true` | `true` | `false` | plain number only | + +**files changed:** + +| file | change | +|---|---| +| `projects/v3/src/app/components/assessment/assessment.component.html` | both pagination blocks: `[class.completed]` and `*ngIf` on checkmark icon now also check `(doAssessment \|\| isPendingReview)` | + +### 2 — scroll reset on page navigation + +**problem:** navigating between pages of different lengths left the scroll position anchored at the bottom, stranding the user mid-page on the new content. + +**decision:** a `_scrollToTop()` method was added and called inside `goToPage()`. it handles two scroll environments: + +| environment | scroll mechanism | +|---|---| +| mobile (`ion-content`) | `ion-content.scrollToTop(0)` via Ionic's native API | +| desktop (split-pane `ion-col.border-left`) | `app-assessment .main-content scrollIntoView({ block: 'start' })` | + +the method prefers the mobile path (returns early on success) so both environments are covered without conflict. + +```typescript +private _scrollToTop() { + setTimeout(() => { + const content = document.querySelector( + 'ion-router-outlet .ion-page:not(.ion-page-hidden) ion-content' + ) || document.querySelector('ion-content'); + if (content && typeof (content as any).scrollToTop === 'function') { + (content as any).scrollToTop(0); + return; + } + const mainContent = + document.querySelector('app-assessment .main-content') || + document.querySelector('app-assessment'); + if (mainContent) { + mainContent.scrollIntoView({ behavior: 'smooth', block: 'start' }); + } else { + window.scrollTo({ top: 0, behavior: 'smooth' }); + } + }, 10); +} +``` + +**files changed:** + +| file | change | +|---|---| +| `projects/v3/src/app/components/assessment/assessment.component.ts` | `_scrollToTop()` private method added; called inside `goToPage()` | + +### 3 — pagination layout: reverted to bottom action bar + +**context:** pagination was temporarily moved into the scrollable `.main-content` body to explore an alternative layout. user feedback indicated this worsened the UX because the controls appeared inconsistently depending on page length and scroll position. + +**decision:** reverted. pagination lives back inside `` with `[hasCustomContent]="isPaginationEnabled && pageCount > 1"`. the `standalone-pagination-footer` `` is also restored for completed reviews where the standard action bar is hidden. + +**files changed:** + +| file | change | +|---|---| +| `projects/v3/src/app/components/assessment/assessment.component.html` | pagination `
` moved back inside ``; `[hasCustomContent]` re-added; `standalone-pagination-footer` `` restored | +| `projects/v3/src/app/components/assessment/assessment.component.scss` | `.standalone-pagination-footer` styles restored | diff --git a/projects/v3/src/app/components/assessment/assessment.component.html b/projects/v3/src/app/components/assessment/assessment.component.html index 71aa7697c..c9d7c55a8 100644 --- a/projects/v3/src/app/components/assessment/assessment.component.html +++ b/projects/v3/src/app/components/assessment/assessment.component.html @@ -421,7 +421,6 @@ -
+ +
+ {{ team360PagesVisited }} of {{ team360MinPages }} member{{ team360MinPages > 1 ? 's' : '' }} reviewed +
+ +
+ *ngIf="isPaginationEnabled && pageCount > 1">
+ tabindex="0" + role="button" + [attr.aria-label]="'Page ' + (i + 1)" + (keydown.enter)="goToPage(page)" + (keydown.space)="goToPage(page); $event.preventDefault()">
{{ i + 1 }}
- - -
@@ -494,6 +498,8 @@
+ + @@ -515,12 +521,11 @@
{{ i + 1 }}
- - -
@@ -554,6 +555,7 @@ + diff --git a/projects/v3/src/app/components/assessment/assessment.component.scss b/projects/v3/src/app/components/assessment/assessment.component.scss index 88b3625e2..bcb59c40a 100644 --- a/projects/v3/src/app/components/assessment/assessment.component.scss +++ b/projects/v3/src/app/components/assessment/assessment.component.scss @@ -218,17 +218,18 @@ ion-footer { // styles for pagination section .pagination-container { - display: flex; + display: inline-flex; align-items: center; - justify-content: space-between; - width: 100%; - min-width: 70%; + justify-content: center; + width: fit-content; + max-width: 100%; padding: 8px 0px; background-color: transparent; box-shadow: none; + gap: 4px; &.few-pages { - justify-content: space-evenly; + justify-content: center; } } @@ -317,15 +318,6 @@ ion-footer { border-color: #81c784; } -.page-indicator.incompleted { - background-color: rgba(255, 0, 0, 0.05); - border-color: var(--ion-color-danger); -} - -.page-indicator.incompleted .page-number { - color: var(--ion-color-danger); -} - .progress-ring { position: relative; display: flex; @@ -365,10 +357,6 @@ ion-footer { stroke-width: 4; } -.page-indicator.incompleted .progress-ring-fill { - stroke: var(--ion-color-danger); -} - .page-number { font-size: 14px; font-weight: 600; @@ -394,10 +382,6 @@ ion-footer { justify-content: center; } -.completion-icon:not(.completed) { - color: var(--ion-color-danger); -} - @media (max-width: 576px) { .pagination-container { padding: 0; @@ -433,17 +417,12 @@ ion-footer { } } -// space for submit button (mid to large screens) — only inside bottom-action-bar -@media (min-width: 769px) and (max-width: 1344px) { - app-bottom-action-bar .pagination-container { - max-width: calc(100% - 150px); - } -} - -@media (min-width: 1345px) { - app-bottom-action-bar .pagination-container { - max-width: calc(100% - 40px); - } +// team 360 progress indicator shown above pagination +.team360-progress { + font-size: 0.75rem; + color: var(--ion-color-medium); + text-align: center; + padding: 2px 0 4px; } // standalone pagination footer for completed reviews diff --git a/projects/v3/src/app/components/assessment/assessment.component.spec.ts b/projects/v3/src/app/components/assessment/assessment.component.spec.ts index d58002a02..296c72550 100644 --- a/projects/v3/src/app/components/assessment/assessment.component.spec.ts +++ b/projects/v3/src/app/components/assessment/assessment.component.spec.ts @@ -1224,7 +1224,7 @@ describe('AssessmentComponent', () => { }); describe('continueToNextTask()', () => { - it('should submit assessment', () => { + it('should submit assessment', async () => { component.doAssessment = true; expect(component.btnText).toEqual('submit answers'); @@ -1233,11 +1233,11 @@ describe('AssessmentComponent', () => { // continueToNextTask pushes to submitActions, which then triggers _submitAnswer via subscription const spy = spyOn(component.submitActions, 'next'); - component.continueToNextTask(); + await component.continueToNextTask(); expect(spy).toHaveBeenCalledWith({ autoSave: false, goBack: false }); }); - it('should mark feedback as read', () => { + it('should mark feedback as read', async () => { component.submission = mockSubmission as any; component.submission.status = 'published'; component.feedbackReviewed = false; @@ -1249,17 +1249,17 @@ describe('AssessmentComponent', () => { expect(component.btnText).toEqual('mark feedback as reviewed'); const spy = spyOn(component.readFeedback, 'emit'); - component.continueToNextTask(); + await component.continueToNextTask(); expect(spy).toHaveBeenCalled(); }); - it('should emit continue', () => { + it('should emit continue', async () => { component.submission = mockSubmission as any; component.submission.status = 'done'; expect(component.btnText).toEqual('continue'); const spy = spyOn(component.continue, 'emit'); - component.continueToNextTask(); + await component.continueToNextTask(); expect(spy).toHaveBeenCalled(); }); }); @@ -1577,35 +1577,35 @@ describe('AssessmentComponent', () => { }); describe('continueToNextTask()', () => { - it('should set submitting=true and disable button on submit', () => { + it('should set submitting=true and disable button on submit', async () => { component.doAssessment = true; const submitSpy = spyOn(component.submitActions, 'next'); - component.continueToNextTask(); + await component.continueToNextTask(); expect(component['submitting']).toBeTrue(); expect(component.btnDisabled$.getValue()).toBeTrue(); expect(submitSpy).toHaveBeenCalledWith({ autoSave: false, goBack: false }); }); - it('should not set submitting flag for readFeedback action', () => { + it('should not set submitting flag for readFeedback action', async () => { component.doAssessment = false; component.submission = { ...mockSubmission, status: 'published', isLocked: false } as any; component.feedbackReviewed = false; const readFeedbackSpy = spyOn(component.readFeedback, 'emit'); - component.continueToNextTask(); + await component.continueToNextTask(); expect(component['submitting']).toBeFalse(); expect(readFeedbackSpy).toHaveBeenCalled(); }); - it('should not set submitting flag for continue action', () => { + it('should not set submitting flag for continue action', async () => { component.doAssessment = false; component.submission = { ...mockSubmission, status: 'done', isLocked: false } as any; const continueSpy = spyOn(component.continue, 'emit'); - component.continueToNextTask(); + await component.continueToNextTask(); expect(component['submitting']).toBeFalse(); expect(continueSpy).toHaveBeenCalled(); @@ -2080,6 +2080,8 @@ describe('AssessmentComponent', () => { tick(200); expect(component.pageRequiredCompletion).toEqual([true, true]); + // all pages marked visited in read-only mode + expect(component.pageVisited).toEqual([true, true]); expect(component.scrollActivePageIntoView).toHaveBeenCalled(); })); @@ -2107,8 +2109,37 @@ describe('AssessmentComponent', () => { expect(component.pageRequiredCompletion[0]).toBeTrue(); // page 1 has unanswered required question → false expect(component.pageRequiredCompletion[1]).toBeFalse(); + // page 0 should be visited (first page), page 1 not yet + expect(component.pageVisited[0]).toBeTrue(); + expect(component.pageVisited[1]).toBeFalse(); expect(component.scrollActivePageIntoView).toHaveBeenCalled(); })); + + it('should preserve existing pageVisited state across re-runs', fakeAsync(() => { + spyOnProperty(component, 'isPaginationEnabled').and.returnValue(true); + component.doAssessment = true; + component.action = 'assessment'; + component.pagesGroups = [ + [{ name: 'G1', questions: [ + { id: 1, name: 'Q1', type: 'text', isRequired: true, audience: ['submitter'] } as any, + ] }], + [{ name: 'G2', questions: [ + { id: 2, name: 'Q2', type: 'text', isRequired: false, audience: ['submitter'] } as any, + ] }], + ]; + component.questionsForm = new FormGroup({ + 'q-1': new FormControl('answered'), + 'q-2': new FormControl('answered'), + }); + // simulate user already visited page 1 + component.pageVisited = [true, true]; + + component.initializePageCompletion(); + tick(200); + + // visited state is preserved (not reset) on re-run + expect(component.pageVisited).toEqual([true, true]); + })); }); describe('findAndGoToFirstUnansweredQuestion()', () => { @@ -2616,6 +2647,7 @@ describe('AssessmentComponent', () => { spyOnProperty(component, 'isPaginationEnabled').and.returnValue(true); component.pagesGroups = [[], [], []]; component.pageIndex = 1; + component.pageVisited = [false, true, false]; spyOn(component, 'scrollActivePageIntoView'); }); @@ -2625,6 +2657,11 @@ describe('AssessmentComponent', () => { expect(component.scrollActivePageIntoView).toHaveBeenCalled(); }); + it('prevPage should mark the destination page as visited', () => { + component.prevPage(); + expect(component.pageVisited[0]).toBeTrue(); + }); + it('prevPage should not go below 0', () => { component.pageIndex = 0; component.prevPage(); @@ -2638,6 +2675,11 @@ describe('AssessmentComponent', () => { expect(component.scrollActivePageIntoView).toHaveBeenCalled(); }); + it('nextPage should mark the destination page as visited', () => { + component.nextPage(); + expect(component.pageVisited[2]).toBeTrue(); + }); + it('nextPage should not exceed last page', () => { component.pageIndex = 2; component.nextPage(); @@ -2666,6 +2708,7 @@ describe('AssessmentComponent', () => { beforeEach(() => { spyOnProperty(component, 'isPaginationEnabled').and.returnValue(true); component.pagesGroups = [[], [], [], []]; + component.pageVisited = [true, false, false, false]; spyOn(component, 'scrollActivePageIntoView'); }); @@ -2675,6 +2718,11 @@ describe('AssessmentComponent', () => { expect(component.scrollActivePageIntoView).toHaveBeenCalled(); }); + it('should mark the target page as visited', () => { + component.goToPage(2); + expect(component.pageVisited[2]).toBeTrue(); + }); + it('should reject negative page index', () => { component.pageIndex = 1; component.goToPage(-1); @@ -2811,6 +2859,441 @@ describe('AssessmentComponent', () => { }); }); + describe('Team 360 minimum pages enforcement', () => { + // group with a team member selector (one group = one team member by design) + const selectorGroup = (id: number) => ({ + name: `Selector Group ${id}`, + description: '', + questions: [{ + id, + type: 'team member selector', + isRequired: false, + audience: ['submitter'], + teamMembers: [{ key: `{"userId":${id}}`, userName: `User ${id}` }], + } as any], + }); + + // group with a multi-member selector (all team members listed as options) + const multiSelectorGroup = (id: number) => ({ + name: `Multi Group ${id}`, + description: '', + questions: [{ + id, + type: 'multi team member selector', + isRequired: false, + audience: ['submitter'], + teamMembers: [ + { key: '{"userId":1}', userName: 'U1' }, + { key: '{"userId":2}', userName: 'U2' }, + { key: '{"userId":3}', userName: 'U3' }, + ], + } as any], + }); + + // group without any selector (self-reflection or plain text) + const textGroup = (id: number) => ({ + name: `Text Group ${id}`, + description: '', + questions: [{ id, type: 'text', isRequired: false, audience: ['submitter'] } as any], + }); + + const makeValidForm = () => new FormGroup({ 'q-100': new FormControl('selected') }); + + beforeEach(() => { + component.btnDisabled$ = new BehaviorSubject(false); + component.doAssessment = true; + component['submitting'] = false; + }); + + describe('team360MinPages getter', () => { + it('returns 0 for non-team-360 task', () => { + component.task = { assessmentType: 'normal' } as any; + const g0 = textGroup(10), g1 = selectorGroup(20), g2 = selectorGroup(100); + component.assessment = { groups: [g0, g1, g2] } as any; + expect(component.team360MinPages).toBe(0); + }); + + it('returns 0 when task is undefined', () => { + component.task = undefined; + component.assessment = { groups: [textGroup(10), selectorGroup(20)] } as any; + expect(component.team360MinPages).toBe(0); + }); + + it('returns 0 when assessment has no groups', () => { + component.task = { assessmentType: 'team360' } as any; + component.assessment = { groups: [] } as any; + expect(component.team360MinPages).toBe(0); + }); + + it('group 0 (self) is excluded — selector groups from index 1 are counted', () => { + component.task = { assessmentType: 'team360' } as any; + component.assessment = { groups: [textGroup(10), selectorGroup(20), selectorGroup(100)] } as any; + expect(component.team360MinPages).toBe(2); + }); + + it('counts multi-member selector groups: distinct member keys, not group count', () => { + component.task = { assessmentType: 'team360' } as any; + // multiSelectorGroup(100) has 3 members (U1, U2, U3) → 3 distinct keys + component.assessment = { groups: [textGroup(10), multiSelectorGroup(100)] } as any; + expect(component.team360MinPages).toBe(3); + }); + + it('deduplication: multiple groups with same member count as 1 (the live-data bug)', () => { + // actual scenario: 4 non-self groups all show the same 1 team member (e.g. test data with + // only learner 004 on the team). minPages should be 1, not 4. + component.task = { assessmentType: 'team360' } as any; + const sameMember = (id: number) => ({ + name: `Group ${id}`, + description: '', + questions: [{ + id, + type: 'team member selector', + isRequired: false, + audience: ['submitter'], + teamMembers: [{ key: '{"userId":4}', userName: 'learner 004' }], + } as any], + }); + component.assessment = { + groups: [textGroup(10), sameMember(20), sameMember(21), sameMember(22), sameMember(23)], + } as any; + expect(component.team360MinPages).toBe(1); // 1 distinct member, not 4 groups + }); + + it('does not count groups without selector questions', () => { + component.task = { assessmentType: 'team360' } as any; + component.assessment = { groups: [textGroup(10), textGroup(20), textGroup(30)] } as any; + expect(component.team360MinPages).toBe(0); + }); + + it('4-teammate scenario: 1 self + 4 selector groups → minPages = 4', () => { + component.task = { assessmentType: 'team360' } as any; + component.assessment = { + groups: [textGroup(10), selectorGroup(20), selectorGroup(100), selectorGroup(101), selectorGroup(102)], + } as any; + expect(component.team360MinPages).toBe(4); + }); + }); + + describe('team360PagesVisited getter', () => { + it('returns 0 for non-team-360 task', () => { + component.task = { assessmentType: 'normal' } as any; + const g0 = textGroup(10), g1 = selectorGroup(20), g2 = selectorGroup(100); + component.assessment = { groups: [g0, g1, g2] } as any; + component.pagesGroups = [[g0], [g1], [g2]]; + component.pageVisited = [true, true, true]; + expect(component.team360PagesVisited).toBe(0); + }); + + it('does not count group 0 (self) even when its page is visited', () => { + component.task = { assessmentType: 'team360' } as any; + const g0 = textGroup(10), g1 = selectorGroup(20), g2 = selectorGroup(100); + component.assessment = { groups: [g0, g1, g2] } as any; + component.pagesGroups = [[g0], [g1], [g2]]; + component.pageVisited = [true, false, false]; + expect(component.team360PagesVisited).toBe(0); + }); + + it('increments by 1 per visited selector group page (1:1 layout)', () => { + component.task = { assessmentType: 'team360' } as any; + const g0 = textGroup(10), g1 = selectorGroup(20), g2 = selectorGroup(100), g3 = selectorGroup(101); + component.assessment = { groups: [g0, g1, g2, g3] } as any; + component.pagesGroups = [[g0], [g1], [g2], [g3]]; + component.pageVisited = [true, true, true, false]; + expect(component.team360PagesVisited).toBe(2); + }); + + it('caps at team360MinPages', () => { + component.task = { assessmentType: 'team360' } as any; + const g0 = textGroup(10), g1 = selectorGroup(20), g2 = selectorGroup(100); + component.assessment = { groups: [g0, g1, g2] } as any; + component.pagesGroups = [[g0], [g1], [g2]]; + component.pageVisited = [true, true, true, true]; // extra entries beyond cap + expect(component.team360PagesVisited).toBe(2); + }); + + it('batching scenario: two selector groups on same page counts 2 when visited', () => { + // splitGroupsByQuestionCount can batch small groups onto one page + component.task = { assessmentType: 'team360' } as any; + const g0 = textGroup(10), g1 = selectorGroup(20), g2 = selectorGroup(100); + component.assessment = { groups: [g0, g1, g2] } as any; + // g1 and g2 batched onto page 1 (e.g. 2 questions total ≤ pageSize) + component.pagesGroups = [[g0], [g1, g2]]; + component.pageVisited = [true, true]; + expect(component.team360PagesVisited).toBe(2); // both groups counted, not just 1 + }); + + it('batching scenario: page not yet visited → 0 even if groups exist', () => { + component.task = { assessmentType: 'team360' } as any; + const g0 = textGroup(10), g1 = selectorGroup(20), g2 = selectorGroup(100); + component.assessment = { groups: [g0, g1, g2] } as any; + component.pagesGroups = [[g0], [g1, g2]]; + component.pageVisited = [true, false]; // page 1 not visited yet + expect(component.team360PagesVisited).toBe(0); + }); + + it('"1st try" scenario: 5 groups with unique single-member selectors, increments per visit', () => { + // real design: each non-self group has 1 specific member (unique key per group). + // visiting each page increments count by 1. group 0 excluded (index 0). + component.task = { assessmentType: 'team360' } as any; + const groups = Array.from({ length: 5 }, (_, i) => selectorGroup(100 + i)); + // keys: {"userId":100} (excluded), {"userId":101}, {"userId":102}, {"userId":103}, {"userId":104} + // minPages = 4 distinct members in groups 1-4 + component.assessment = { groups } as any; + component.pagesGroups = groups.map(g => [g]); + + component.pageVisited = [true, true, false, false, false]; + expect(component.team360PagesVisited).toBe(1); // group 1 visited → member 101 → 1 of 4 + + component.pageVisited = [true, true, true, false, false]; + expect(component.team360PagesVisited).toBe(2); // groups 1-2 → 2 of 4 + + component.pageVisited = [true, true, true, true, true]; + expect(component.team360PagesVisited).toBe(4); // groups 1-4 → 4 of 4 (capped at minPages) + }); + + it('deduplication: 4 groups same member → visiting any one marks 1 of 1 reviewed', () => { + // the live-data scenario: 4 non-self groups all showing learner 004 + component.task = { assessmentType: 'team360' } as any; + const sameMember = (id: number) => ({ + name: `Group ${id}`, + description: '', + questions: [{ + id, + type: 'team member selector', + isRequired: false, + audience: ['submitter'], + teamMembers: [{ key: '{"userId":4}', userName: 'learner 004' }], + } as any], + }); + const g0 = textGroup(10), g1 = sameMember(20), g2 = sameMember(21), g3 = sameMember(22), g4 = sameMember(23); + component.assessment = { groups: [g0, g1, g2, g3, g4] } as any; + component.pagesGroups = [[g0], [g1], [g2], [g3], [g4]]; + + component.pageVisited = [true, true, false, false, false]; + expect(component.team360PagesVisited).toBe(1); // visited group 1 → learner 004 → 1 of 1 + + component.pageVisited = [true, false, false, false, false]; + expect(component.team360PagesVisited).toBe(0); // only self page visited → 0 of 1 + }); + + it('3-member team: each group lists ALL members as selector options — only visited groups count', () => { + // bug scenario: "2nd try" data — 3 people, reviewing 2. + // each selector question lists ALL team members as options. + // old key-based visited counting instantly showed "2 of 2" when visiting group 1 + // because both keys were added to the visited set from that one group's teamMembers. + // new group-count approach: visiting group 1 → 1 of 2 (not 2 of 2). + component.task = { assessmentType: 'team360' } as any; + const allMemberKeys = [ + { key: '{"userId":1}', userName: 'Member 1' }, + { key: '{"userId":2}', userName: 'Member 2' }, + ]; + const makeGroupAllMembers = (id: number) => ({ + name: `Group ${id}`, + description: '', + questions: [{ + id, + type: 'team member selector', + isRequired: false, + audience: ['submitter'], + teamMembers: allMemberKeys, + } as any], + }); + const g0 = textGroup(10), g1 = makeGroupAllMembers(20), g2 = makeGroupAllMembers(21); + component.assessment = { groups: [g0, g1, g2] } as any; + component.pagesGroups = [[g0], [g1], [g2]]; + + // min uses distinct keys: 2 members listed across non-self groups + expect(component.team360MinPages).toBe(2); + + component.pageVisited = [true, false, false]; + expect(component.team360PagesVisited).toBe(0); // only self visited + + component.pageVisited = [true, true, false]; + expect(component.team360PagesVisited).toBe(1); // group 1 visited — NOT instantly 2 + + component.pageVisited = [true, true, true]; + expect(component.team360PagesVisited).toBe(2); // both groups visited → 2 of 2 + }); + }); + + describe('setSubmissionDisabled() team 360 enforcement', () => { + it('keeps button disabled when form valid but not all team member pages visited', () => { + component.task = { assessmentType: 'team360' } as any; + const g0 = textGroup(10), g1 = selectorGroup(20), g2 = selectorGroup(100), g3 = selectorGroup(101); + component.assessment = { groups: [g0, g1, g2, g3] } as any; + component.pagesGroups = [[g0], [g1], [g2], [g3]]; + component.questionsForm = makeValidForm(); + component.pageVisited = [true, false, false, false]; + component.btnDisabled$ = new BehaviorSubject(true); + + component.setSubmissionDisabled(); + + expect(component.btnDisabled$.getValue()).toBeTrue(); + }); + + it('enables button when form valid and all team member pages visited', () => { + component.task = { assessmentType: 'team360' } as any; + const g0 = textGroup(10), g1 = selectorGroup(20), g2 = selectorGroup(100), g3 = selectorGroup(101); + component.assessment = { groups: [g0, g1, g2, g3] } as any; + component.pagesGroups = [[g0], [g1], [g2], [g3]]; + component.questionsForm = makeValidForm(); + component.pageVisited = [true, true, true, true]; + component.btnDisabled$ = new BehaviorSubject(true); + + component.setSubmissionDisabled(); + + expect(component.btnDisabled$.getValue()).toBeFalse(); + }); + + it('no enforcement when no selector groups exist after index 0', () => { + component.task = { assessmentType: 'team360' } as any; + const g0 = textGroup(10), g1 = textGroup(20), g2 = textGroup(30); + component.assessment = { groups: [g0, g1, g2] } as any; + component.pagesGroups = [[g0], [g1], [g2]]; + component.questionsForm = makeValidForm(); + component.pageVisited = [false, false, false]; + component.btnDisabled$ = new BehaviorSubject(true); + + component.setSubmissionDisabled(); + + expect(component.btnDisabled$.getValue()).toBeFalse(); + }); + + it('does not apply enforcement for non-team-360 assessments', () => { + component.task = { assessmentType: 'normal' } as any; + const g0 = textGroup(10), g1 = selectorGroup(20), g2 = selectorGroup(100); + component.assessment = { groups: [g0, g1, g2] } as any; + component.pagesGroups = [[g0], [g1], [g2]]; + component.questionsForm = makeValidForm(); + component.pageVisited = [false, false, false]; + component.btnDisabled$ = new BehaviorSubject(true); + + component.setSubmissionDisabled(); + + expect(component.btnDisabled$.getValue()).toBeFalse(); + }); + + it('still disables when form invalid even if all pages visited', () => { + component.task = { assessmentType: 'team360' } as any; + const g0 = textGroup(10), g1 = selectorGroup(20), g2 = selectorGroup(100); + component.assessment = { groups: [g0, g1, g2] } as any; + component.pagesGroups = [[g0], [g1], [g2]]; + component.questionsForm = new FormGroup({ 'q-100': new FormControl(null, Validators.required) }); + component.pageVisited = [true, true, true]; + component.btnDisabled$ = new BehaviorSubject(false); + + component.setSubmissionDisabled(); + + expect(component.btnDisabled$.getValue()).toBeTrue(); + }); + + it('batching scenario: visiting batched page enables submit when form valid', () => { + // g1 and g2 on same page — visiting page 1 satisfies both teammates + component.task = { assessmentType: 'team360' } as any; + const g0 = textGroup(10), g1 = selectorGroup(20), g2 = selectorGroup(100); + component.assessment = { groups: [g0, g1, g2] } as any; + component.pagesGroups = [[g0], [g1, g2]]; + component.questionsForm = makeValidForm(); + component.pageVisited = [true, true]; + component.btnDisabled$ = new BehaviorSubject(true); + + component.setSubmissionDisabled(); + + expect(component.btnDisabled$.getValue()).toBeFalse(); + }); + + it('"1st try" scenario: 5 groups unique members → increments per visit, enables at 4 of 4', () => { + component.task = { assessmentType: 'team360' } as any; + const groups = Array.from({ length: 5 }, (_, i) => selectorGroup(100 + i)); + // group 0 (key {"userId":100}) excluded; groups 1-4 have unique keys → minPages = 4 + component.assessment = { groups } as any; + component.pagesGroups = groups.map(g => [g]); + component.questionsForm = makeValidForm(); + component.btnDisabled$ = new BehaviorSubject(true); + + component.pageVisited = [true, true, false, false, false]; // 1 of 4 + component.setSubmissionDisabled(); + expect(component.btnDisabled$.getValue()).toBeTrue(); + + component.pageVisited = [true, true, true, true, false]; // 3 of 4 + component.setSubmissionDisabled(); + expect(component.btnDisabled$.getValue()).toBeTrue(); + + component.pageVisited = [true, true, true, true, true]; // 4 of 4 + component.setSubmissionDisabled(); + expect(component.btnDisabled$.getValue()).toBeFalse(); + }); + + it('deduplication: 4 groups same member → visiting 1 page enables submit', () => { + // actual live-data bug: 4 non-self groups all showing learner 004. should need only 1 page + // visit, not 4. + component.task = { assessmentType: 'team360' } as any; + const sameMember = (id: number) => ({ + name: `Group ${id}`, + description: '', + questions: [{ + id, + type: 'team member selector', + isRequired: false, + audience: ['submitter'], + teamMembers: [{ key: '{"userId":4}', userName: 'learner 004' }], + } as any], + }); + const g0 = textGroup(10), g1 = sameMember(20), g2 = sameMember(21), g3 = sameMember(22), g4 = sameMember(23); + component.assessment = { groups: [g0, g1, g2, g3, g4] } as any; + component.pagesGroups = [[g0], [g1], [g2], [g3], [g4]]; + component.questionsForm = makeValidForm(); + component.btnDisabled$ = new BehaviorSubject(true); + + // only self page visited — not enough + component.pageVisited = [true, false, false, false, false]; + component.setSubmissionDisabled(); + expect(component.btnDisabled$.getValue()).toBeTrue(); + + // visit any one non-self page → learner 004 reviewed → 1 of 1 → enabled + component.pageVisited = [true, true, false, false, false]; + component.setSubmissionDisabled(); + expect(component.btnDisabled$.getValue()).toBeFalse(); + }); + + it('3-member team: all-members selector does not instantly enable submit after first page visit', () => { + // regression for bug: visiting page 1 (g1) was adding both member keys to visited set + // → instantly showing "2 of 2" → enabling submit prematurely. + component.task = { assessmentType: 'team360' } as any; + const allMemberKeys = [ + { key: '{"userId":1}', userName: 'Member 1' }, + { key: '{"userId":2}', userName: 'Member 2' }, + ]; + const makeGroupAllMembers = (id: number) => ({ + name: `Group ${id}`, + description: '', + questions: [{ + id, + type: 'team member selector', + isRequired: false, + audience: ['submitter'], + teamMembers: allMemberKeys, + } as any], + }); + const g0 = textGroup(10), g1 = makeGroupAllMembers(20), g2 = makeGroupAllMembers(21); + component.assessment = { groups: [g0, g1, g2] } as any; + component.pagesGroups = [[g0], [g1], [g2]]; + component.questionsForm = makeValidForm(); + component.btnDisabled$ = new BehaviorSubject(true); + + // visit only page 1 (g1) — should be 1 of 2, NOT 2 of 2 + component.pageVisited = [true, true, false]; + component.setSubmissionDisabled(); + expect(component.btnDisabled$.getValue()).toBeTrue(); // still disabled + + // visit both team member pages → 2 of 2 → enabled + component.pageVisited = [true, true, true]; + component.setSubmissionDisabled(); + expect(component.btnDisabled$.getValue()).toBeFalse(); + }); + }); + }); + describe('_prefillForm() with locked submission', () => { it('should keep button disabled when submission is locked', () => { component.questionsForm = new FormGroup({ diff --git a/projects/v3/src/app/components/assessment/assessment.component.ts b/projects/v3/src/app/components/assessment/assessment.component.ts index 702e088dd..0feb5800e 100644 --- a/projects/v3/src/app/components/assessment/assessment.component.ts +++ b/projects/v3/src/app/components/assessment/assessment.component.ts @@ -1,6 +1,6 @@ import { environment } from '@v3/environments/environment'; import { Component, Input, Output, EventEmitter, OnChanges, OnDestroy, OnInit, QueryList, ViewChildren, ChangeDetectionStrategy, ViewChild, signal, ElementRef, SimpleChanges, ChangeDetectorRef } from '@angular/core'; -import { Assessment, Submission, AssessmentReview, AssessmentSubmitParams, AssessmentService } from '@v3/services/assessment.service'; +import { Assessment, Group, Submission, AssessmentReview, AssessmentSubmitParams, AssessmentService } from '@v3/services/assessment.service'; import { UtilsService } from '@v3/services/utils.service'; import { NotificationsService } from '@v3/services/notifications.service'; import { FormGroup, FormControl, Validators } from '@angular/forms'; @@ -137,6 +137,7 @@ export class AssessmentComponent implements OnInit, OnChanges, OnDestroy { // pagination pageRequiredCompletion: boolean[] = []; // indicator for required questions + pageVisited: boolean[] = []; // tracks which pages the user has visited readonly manyPages = MIN_SCROLLING_PAGES; @ViewChildren('questionBox') questionBoxes!: QueryList<{ el: HTMLElement }>; @@ -197,6 +198,7 @@ export class AssessmentComponent implements OnInit, OnChanges, OnDestroy { if (!this.isPaginationEnabled) return; if (this.pageIndex > 0) { this.pageIndex--; + this.pageVisited[this.pageIndex] = true; this.scrollActivePageIntoView(); this.setSubmissionDisabled(); } @@ -206,6 +208,7 @@ export class AssessmentComponent implements OnInit, OnChanges, OnDestroy { if (!this.isPaginationEnabled) return; if (this.pageIndex < this.pageCount - 1) { this.pageIndex++; + this.pageVisited[this.pageIndex] = true; this.scrollActivePageIntoView(); this.setSubmissionDisabled(); } @@ -219,12 +222,35 @@ export class AssessmentComponent implements OnInit, OnChanges, OnDestroy { goToPage(i: number) { if (!this.isPaginationEnabled) return; if (i >= 0 && i < this.pageCount) { - this.pageIndex = i; - this.scrollActivePageIntoView(); - this.setSubmissionDisabled(); + if (this.pageIndex !== i) { + this.pageIndex = i; + this.pageVisited[i] = true; + this.scrollActivePageIntoView(); + this.setSubmissionDisabled(); + this._scrollToTop(); + } } } + private _scrollToTop() { + setTimeout(() => { + // 1. Try standard ion-content on mobile + const content = document.querySelector('ion-router-outlet .ion-page:not(.ion-page-hidden) ion-content') || document.querySelector('ion-content'); + if (content && typeof (content as any).scrollToTop === 'function') { + (content as any).scrollToTop(0); + return; + } + + // 2. Scroll the main content into view (handles desktop ion-col scroll containers) + const mainContent = document.querySelector('app-assessment .main-content') || document.querySelector('app-assessment'); + if (mainContent) { + mainContent.scrollIntoView({ behavior: 'smooth', block: 'start' }); + } else { + window.scrollTo({ top: 0, behavior: 'smooth' }); + } + }, 10); + } + ngOnInit(): void { this.subscribeSaveSubmission(); } @@ -427,6 +453,7 @@ Best regards`; if (changes.assessment || changes.submission || changes.review) { this.pageRequiredCompletion = []; + this.pageVisited = []; this._handleSubmissionData(); @@ -701,13 +728,10 @@ Best regards`; */ continueToNextTask() { switch (this._btnAction) { - case 'submit': - this.submitting = true; - this.btnDisabled$.next(true); - return this.submitActions.next({ - autoSave: false, - goBack: false, - }); + case 'submit': { + this._doSubmit(); + return; + } case 'readFeedback': this.btnDisabled$.next(true); return this.readFeedback.emit(this.submission.id); @@ -716,6 +740,16 @@ Best regards`; } } + /** fires the actual submission — extracted so the alert guard can call it after confirmation */ + private _doSubmit() { + this.submitting = true; + this.btnDisabled$.next(true); + this.submitActions.next({ + autoSave: false, + goBack: false, + }); + } + /** * @name filledAnswers * @description to collect all latest answers from the form @@ -1099,6 +1133,8 @@ Best regards`; // read-only mode (viewing feedback or completed submissions), completion tracking is not relevant if (!this.doAssessment && !this.isPendingReview) { this.pageRequiredCompletion = new Array(this.pageCount).fill(true); + // mark all pages visited in read-only so indicators are all visible + this.pageVisited = new Array(this.pageCount).fill(true); this.cdr.detectChanges(); setTimeout(() => this.scrollActivePageIntoView(), 100); return; @@ -1106,6 +1142,12 @@ Best regards`; this.pageRequiredCompletion = new Array(this.pageCount).fill(true); + // initialise visited array if not yet created; always mark page 0 as visited on first load + if (!this.pageVisited.length) { + this.pageVisited = new Array(this.pageCount).fill(false); + this.pageVisited[0] = true; + } + this.pages.forEach((page, index) => { const pageQuestions = this.getAllQuestionsForPage(index); this.pageRequiredCompletion[index] = this.areAllRequiredQuestionsAnswered(pageQuestions); @@ -1331,16 +1373,78 @@ Best regards`; } const isFormValid = this.questionsForm?.valid ?? false; + const minPages = this._team360MinPages; + const visitedEnough = minPages === 0 || this.team360PagesVisited >= minPages; + const shouldDisable = !isFormValid || !visitedEnough; const isCurrentlyDisabled = this.btnDisabled$.getValue(); - // Update button state only if it needs to change - if (!isFormValid && !isCurrentlyDisabled) { + // update button state only if it needs to change + if (shouldDisable && !isCurrentlyDisabled) { this.btnDisabled$.next(true); - } else if (isFormValid && isCurrentlyDisabled) { + } else if (!shouldDisable && isCurrentlyDisabled) { this.btnDisabled$.next(false); } } + /** + * returns the number of distinct team members the user must review in a team 360 assessment. + * counts unique teamMembers.key values across selector questions in groups from index 1+ + * (index 0 is self-reflection, excluded). deduplication ensures that multiple groups referencing + * the same member (e.g. test data where only 1 member exists) show the correct count. + * uses pagesGroups map for safe lookup regardless of how splitGroupsByQuestionCount batches groups. + */ + private get _team360MinPages(): number { + if (this.task?.assessmentType !== 'team360') return 0; + const groups = this.assessment?.groups ?? []; + const memberKeys = new Set(); + // group 0 is self-reflection; team member groups start at index 1 + for (let i = 1; i < groups.length; i++) { + const selectorQ = groups[i].questions?.find(q => + q.type === 'team member selector' || q.type === 'multi team member selector' + ); + if (!selectorQ) continue; + (selectorQ.teamMembers ?? []).forEach((m: { key: string }) => memberKeys.add(m.key)); + } + return memberKeys.size; + } + + /** public accessor for template binding */ + get team360MinPages(): number { + return this._team360MinPages; + } + + /** + * count of distinct team member keys whose pages have been visited. + * for each non-self selector group (index 1+), if its page has been visited, all of its + * teamMembers keys are added to a visited-keys set. deduplication means that visiting any page + * that references a member marks that member as reviewed — enabling correct counts when multiple + * groups share the same member or when groups are batched onto a single page. + */ + get team360PagesVisited(): number { + const min = this._team360MinPages; + if (min === 0) return 0; + const groups = this.assessment?.groups ?? []; + // build a map: group reference → page index + const groupPageIndex = new Map(); + this.pagesGroups.forEach((page, pageIdx) => { + page.forEach(g => groupPageIndex.set(g, pageIdx)); + }); + // count visited selector groups (not visited keys) — capped at min. + // each non-self group = one reviewed person by design, so group visits are the correct unit. + // capping at min handles the case where multiple groups share the same person (e.g. "1st try" + // test data with 4 groups for 1 member: min=1 so cap prevents over-counting). + let count = 0; + for (let i = 1; i < groups.length; i++) { + const hasSelector = groups[i].questions?.some(q => + q.type === 'team member selector' || q.type === 'multi team member selector' + ); + if (!hasSelector) continue; + const pageIdx = groupPageIndex.get(groups[i]); + if (pageIdx !== undefined && this.pageVisited[pageIdx]) count++; + } + return Math.min(count, min); + } + /** * determine if required indicators should be shown for a question * only show required indicators when user can actually edit the form diff --git a/projects/v3/src/app/components/bottom-action-bar/bottom-action-bar.component.html b/projects/v3/src/app/components/bottom-action-bar/bottom-action-bar.component.html index a0971c4c2..8b1e00d4c 100644 --- a/projects/v3/src/app/components/bottom-action-bar/bottom-action-bar.component.html +++ b/projects/v3/src/app/components/bottom-action-bar/bottom-action-bar.component.html @@ -4,7 +4,7 @@

Team assessment submissions restricted to participants only.

-
+
diff --git a/projects/v3/src/app/components/bottom-action-bar/bottom-action-bar.component.scss b/projects/v3/src/app/components/bottom-action-bar/bottom-action-bar.component.scss index 8ff5bdaa1..f676b481c 100644 --- a/projects/v3/src/app/components/bottom-action-bar/bottom-action-bar.component.scss +++ b/projects/v3/src/app/components/bottom-action-bar/bottom-action-bar.component.scss @@ -42,6 +42,11 @@ align-items: center; // Center items vertically width: 100%; justify-content: space-between; // Space between pagination and buttons + + &.stacked { + flex-direction: column; + align-items: center; + } } .custom-content { @@ -61,7 +66,7 @@ &.with-custom-content { margin-top: 0; - justify-content: flex-end; + justify-content: center; } } diff --git a/projects/v3/src/app/pages/activity-desktop/activity-desktop.page.ts b/projects/v3/src/app/pages/activity-desktop/activity-desktop.page.ts index fe24bf120..dacbae7d9 100644 --- a/projects/v3/src/app/pages/activity-desktop/activity-desktop.page.ts +++ b/projects/v3/src/app/pages/activity-desktop/activity-desktop.page.ts @@ -213,11 +213,9 @@ export class ActivityDesktopPage { if (targetTask) { this.goToTask({ + ...targetTask, id: assessmentId || directTaskId, contextId: this.urlParams.contextId, - type: targetTask.type, - name: targetTask.name, - status: targetTask.status, }); } }