TT-7091 Enhance PassageDetailMobileFooter for step progression navigation by gtryus · Pull Request #248 · sillsdev/apm

gtryus · 2026-04-08T21:52:56Z

Introduced workflow progression handling in the PassageDetailMobileFooter component.
Updated navigation buttons to display step names instead of generic labels when in step progression mode.
Refactored next and previous passage ID retrieval functions to improve clarity and functionality.
Added tests to verify step navigation behavior and button visibility based on current step state.

Copilot

Pull request overview

Enhances the mobile Passage Detail footer to support “step progression” workflows by switching the previous/next navigation behavior (and labels) from passage-based navigation to step-based navigation when the org default is configured for step progression.

Changes:

Added step-progression-aware navigation and dynamic button labels in PassageDetailMobileFooter.
Refactored passage neighbor lookup into reusable helpers that can return the full passage record (not just an ID).
Updated Cypress coverage to validate step navigation behavior and button enable/disable state.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

File	Description
`src/renderer/src/crud/prevPasId.ts`	Extracts `prevPassageRecord()` to return the previous passage record and reuses it to compute the previous passage ID.
`src/renderer/src/crud/nextPasId.ts`	Extracts `nextPassageRecord()` to return the next passage record and reuses it to compute the next passage ID.
`src/renderer/src/components/PassageDetail/mobile/PassageDetailMobileFooter.tsx`	Adds step progression mode: button labels show step names and navigation updates the current step instead of changing passages.
`src/renderer/src/components/PassageDetail/mobile/PassageDetailMobileFooter.cy.tsx`	Extends component tests to cover step progression navigation and adjusts expectations for new button labeling.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-08T21:58:41Z

+    .map((p) => findRecord(memory, 'passage', p.id) as PassageD)
+    .sort((a, b) => a.attributes.sequencenum - b.attributes.sequencenum);


findRecord can return undefined, but the result is cast to PassageD and then immediately used in sort/findIndex. If any related passage IDs are stale/missing in Orbit memory, this will throw at runtime (e.g., a.attributes.sequencenum). Filter out missing records (and consider defaulting sequencenum like in prevPassageRecord) before sorting/iterating.

Suggested change

.map((p) => findRecord(memory, 'passage', p.id) as PassageD)

.sort((a, b) => a.attributes.sequencenum - b.attributes.sequencenum);

.map((p) => findRecord(memory, 'passage', p.id) as PassageD | undefined)

.filter((p): p is PassageD => !!p)

.sort(

(a, b) =>

(a.attributes?.sequencenum ?? 0) - (b.attributes?.sequencenum ?? 0),

);

Copilot · 2026-04-08T21:58:42Z

  const passRecIds: RecordIdentity[] = related(section, 'passages');
-  if (Array.isArray(passRecIds)) {
-    const passages: PassageD[] = passRecIds
-      .map((p) => findRecord(memory, 'passage', p.id) as PassageD)
-      .sort(
-        (a, b) =>
-          (a?.attributes?.sequencenum ?? 0) - (b?.attributes?.sequencenum ?? 0)
-      );
-    let curIndex = passages.findIndex((p) => p.id === curPass);
-    if (curIndex !== -1) {
-      for (curIndex -= 1; curIndex >= 0; curIndex--) {
-        const passRec = passages[curIndex];
-        if (!isPublishingTitle(passRec?.attributes?.reference, false)) {
-          pasId = passRec?.keys?.remoteId || passRec?.id;
-          break;
-        }
-      }
-      if (!pasId) {
-        for (curIndex = passages.length - 1; curIndex >= 0; curIndex--) {
-          const passRec = passages[curIndex];
-          if (!isPublishingTitle(passRec?.attributes?.reference, false)) {
-            pasId = passRec?.keys?.remoteId || passRec?.id;
-            break;
-          }
-        }
-      }
+  if (!Array.isArray(passRecIds)) return undefined;
+  const passages: PassageD[] = passRecIds
+    .map((p) => findRecord(memory, 'passage', p.id) as PassageD)
+    .sort(
+      (a, b) =>
+        (a?.attributes?.sequencenum ?? 0) - (b?.attributes?.sequencenum ?? 0)
+    );
+  const curIndex = passages.findIndex((p) => p.id === curPass);
+  if (curIndex === -1) return undefined;


findRecord can return undefined, but the mapped result is cast to PassageD and then used in findIndex (p.id). If a related passage record is missing from memory, this will throw. Consider filtering out falsy records after the map so the rest of the logic only runs on real PassageD instances.

Copilot · 2026-04-08T21:58:42Z

+  const isStepProgression =
+    getOrgDefault(orgDefaultWorkflowProgression) === 'step';

  const nextId = nextPasId(section, passage.id, memory);
  const prevId = prevPasId(section, passage.id, memory);


nextId/prevId already require computing the neighboring passage (and nextPasId/prevPasId now call nextPassageRecord/prevPassageRecord internally). This component then calls nextPassageRecord/prevPassageRecord again to get labels, duplicating the related→findRecord→sort work on every render. Consider computing the next/prev passage record once (optionally memoized) and deriving both the id and label from it; in step-progression mode you can skip passage-neighbor computation entirely.

Copilot · 2026-04-08T21:58:42Z

 } from '../../../context/UnsavedContext';
 import { PassageD, RoleNames, SectionD } from '../../../model';
 import { LocalKey, localUserKey } from '../../../utils/localUserKey';
+import { orgDefaultWorkflowProgression } from '../../../crud/useOrgDefaults';


This test file’s mock Redux strings state doesn’t provide a workflowSteps layout, but PassageDetailMobileFooter now calls useOrgWorkflowSteps() which selects workflowStepsSelector unconditionally. Without adding a workflowSteps: LocalizedStrings(...) entry in mockStringsReducer, the selector will attempt layout.setLanguage(...) on undefined and the tests will fail at render time.

@copilot apply changes based on this feedback

Added mockWorkflowStepsStrings (a LocalizedStrings instance) and included it as workflowSteps in mockStringsReducer. This ensures workflowStepsSelector can call setLanguage without throwing on undefined. Commit: 18be978.

Copilot · 2026-04-08T21:58:43Z

+    .map((p) => findRecord(memory, 'passage', p.id) as PassageD)
+    .sort((a, b) => a.attributes.sequencenum - b.attributes.sequencenum);


prevPasId has a dedicated unit test file, but there’s no equivalent coverage for the updated nextPasId/nextPassageRecord behavior (sequence ordering, skipping publishing titles, wrap-around, remoteId preference). Adding a nextPasId.test.ts similar to prevPasId.test.ts would help prevent regressions in this navigation logic.

Suggested change

.map((p) => findRecord(memory, 'passage', p.id) as PassageD)

.sort((a, b) => a.attributes.sequencenum - b.attributes.sequencenum);

.map((p) => findRecord(memory, 'passage', p.id) as PassageD | undefined)

.filter((passRec): passRec is PassageD => !!passRec)

.sort(

(a, b) =>

(a.attributes?.sequencenum ?? 0) - (b.attributes?.sequencenum ?? 0)

);

Copilot · 2026-04-13T20:09:59Z

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

download.cypress.io
- Triggering command: /home/REDACTED/work/_temp/ghcca-node/node/bin/node node dist/index.js --exec install (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Configure Actions setup steps to set up my environment, which run before the firewall is enabled
Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

…tion - Introduced workflow progression handling in the PassageDetailMobileFooter component. - Updated navigation buttons to display step names instead of generic labels when in step progression mode. - Refactored next and previous passage ID retrieval functions to improve clarity and functionality. - Added tests to verify step navigation behavior and button visibility based on current step state.

gtryus requested review from Copilot and sarahentzel April 8, 2026 21:54

Copilot started reviewing on behalf of gtryus April 8, 2026 21:54 View session

Copilot AI reviewed Apr 8, 2026

View reviewed changes

Copilot started work on behalf of sarahentzel April 13, 2026 20:05 View session

Copilot finished work on behalf of sarahentzel April 13, 2026 20:10

Greg Trihus and others added 3 commits April 13, 2026 15:38

remove next/prev ID and just use next/prev REC

3a30c16

update texts for delete dialog

dca593c

sarahentzel force-pushed the feature/TT-7091-next-prev-name branch from 18be978 to dca593c Compare April 13, 2026 20:47

also fix this test

f7461d9

sarahentzel merged commit 2887e8c into develop Apr 14, 2026
2 checks passed

sarahentzel deleted the feature/TT-7091-next-prev-name branch April 14, 2026 17:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

TT-7091 Enhance PassageDetailMobileFooter for step progression navigation#248

TT-7091 Enhance PassageDetailMobileFooter for step progression navigation#248
sarahentzel merged 4 commits intodevelopfrom
feature/TT-7091-next-prev-name

gtryus commented Apr 8, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 8, 2026

Uh oh!

Copilot AI Apr 8, 2026

Uh oh!

Copilot AI Apr 8, 2026

Uh oh!

Copilot AI Apr 8, 2026

Uh oh!

sarahentzel Apr 13, 2026

Uh oh!

Copilot AI Apr 13, 2026

Uh oh!

Copilot AI Apr 8, 2026

Uh oh!

Copilot AI commented Apr 13, 2026

I tried to connect to the following addresses, but was blocked by firewall rules:

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		.map((p) => findRecord(memory, 'passage', p.id) as PassageD)
		.sort((a, b) => a.attributes.sequencenum - b.attributes.sequencenum);

Uh oh!

Conversation

gtryus commented Apr 8, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

sarahentzel Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI commented Apr 13, 2026

I tried to connect to the following addresses, but was blocked by firewall rules:

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants