[CDX-329] SPA Navigation Guide

[HHHindawy]

Pull Request Checklist

Before you submit a pull request, please make sure you have to following:

• I have added or updated TypeScript types for my changes, ensuring they are compatible with the existing codebase.
• I have added JSDoc comments to my TypeScript definitions for improved documentation.
• I have added tests that prove my fix is effective or that my feature works.
• I have added any necessary documentation (if appropriate).
• I have made sure my PR is up-to-date with the main branch.

PR Type

What kind of change does this PR introduce?

• Bugfix
• Feature
• Code style update (formatting, local variables)
• Refactoring (no functional changes, no API changes)
• Documentation content changes
• TypeScript type definitions update
• Other... Please describe:

[constructor-claude-bedrock]

Code Review Results

✅ Strengths

Well-structured, comprehensive technical documentation guide with clear explanations and practical code examples for implementing SPA navigation.

🚨 Critical Issues

None identified.

⚠️ Important Issues

[File: src/stories/usage-examples/SpaNavigation.mdx Line: L275] - Missing variable declaration in code example
The onRedirect callback in the final example at line 275 references a navigate variable that is not imported or declared in the code snippet. This will cause a runtime error if developers copy-paste the example.

Suggestion: Update the example to show the full context with React Router imports or make it clear this is a fragment by adding a comment.

[File: src/stories/usage-examples/SpaNavigation.mdx Line: L124] - Potential infinite loop with PopStateEvent
The custom popstate dispatch in line 124 may cause an infinite loop since the useHistoryLocation hook listens to popstate events, which would trigger a re-render, potentially calling setUrl again.

Suggestion: Consider documenting this potential issue or using a different approach to trigger React updates.

💡 Suggestions

[File: src/stories/usage-examples/SpaNavigation.mdx Line: L88-89] - Consider adding explanation for the key prop pattern
The comment mentions passing the current URL as the key but this pattern might not be obvious to all developers. Consider adding a note about why this forces remounting.

[File: src/stories/usage-examples/SpaNavigation.mdx Line: L187] - Inconsistent key patterns in examples
The pagination and facets examples use different key naming conventions. Consider standardizing for consistency.

[File: src/stories/usage-examples/SpaNavigation.mdx Line: L242] - Incomplete filter toggle logic
The checkbox onChange handler uses a simplified pattern. Consider adding a comment that production implementations may need more complex logic for multi-select facets.

Overall Assessment: ✅ Pass

This is high-quality documentation that will significantly help developers implementing SPA navigation. The few issues identified are minor and easily addressable.

[constructor-claude-bedrock]

Code Review Results

✅ Strengths

This documentation comprehensively addresses a critical SPA integration issue with clear, working examples for React Router and generic implementations.

🚨 Critical Issues

None found.

⚠️ Important Issues

1. Inconsistent Key Usage Guidance

File: src/stories/usage-examples/SpaNavigation.mdx Lines: L75-L83, L138-L146

The documentation provides two contradictory patterns for handling PLP remounting:

• Lines 75-83: Shows commented-out code suggesting an incremental counter approach
• Lines 138-146: Shows the same incremental counter approach again
• Lines 86 and 148: Demonstrates using currentUrl/href directly as the key

Issue: Using the full URL as a React key can cause problems:

1. Very long URLs may hit browser limits or cause performance issues with React's reconciliation
2. The documentation doesn't clearly recommend which approach to use

Recommendation:

• Choose ONE recommended approach and use it consistently in both examples
• If the counter approach is preferred, don't show it as commented-out code—show it as the primary example
• If the URL-as-key approach is preferred, remove the counter pattern entirely or clearly state when each should be used
• Add a brief explanation of the trade-offs between approaches

2. Missing Information About When Remounting is Necessary

File: src/stories/usage-examples/SpaNavigation.mdx Line: L24

The checklist states "Remount <CioPlp /> on URL changes (required)", but the documentation doesn't explain why this is required.

Issue: Developers need to understand the underlying reason to properly implement this pattern and troubleshoot issues.

Recommendation: Add a brief explanation near line 24 or in Step 1 explaining:

• The library's fetch hooks run on mount
• URL changes in SPA routing don't trigger component remounts by default
• Without remounting, the PLP won't refetch data after URL changes

3. Incomplete Error Handling in Examples

File: src/stories/usage-examples/SpaNavigation.mdx Lines: L55, L129

The setUrl callbacks in both examples don't handle potential errors from:

1. Invalid URLs passed to new URL()
2. Navigation failures in the router

Issue: Production code should handle edge cases gracefully.

Recommendation: Add try-catch blocks or at least a comment indicating error handling should be added in production code.

4. Potential Race Condition in History Listener

File: src/stories/usage-examples/SpaNavigation.mdx Lines: L112-L117

The useHistoryLocation hook manually dispatches a popstate event after pushState, which could cause issues:

1. Multiple rapid navigation calls could lead to race conditions
2. This pattern might interfere with frameworks that listen to popstate

Issue: This is a non-standard pattern that could cause bugs in complex applications.

Recommendation:

• Add a comment warning about potential race conditions
• Suggest using a state-based approach or router library instead of manual pushState when possible
• Consider using useEffect with cleanup to handle multiple rapid updates

5. Missing Accessibility Considerations

File: src/stories/usage-examples/SpaNavigation.mdx Lines: L197-L219, L223-L238, L246-L273

The custom component examples show aria-current and aria-pressed, but don't mention:

1. Focus management after navigation
2. Screen reader announcements for filter/sort changes
3. ARIA live regions for dynamic content updates

Issue: SPA navigation can break accessibility if not properly implemented.

Recommendation: Add a brief section or note about accessibility considerations:

• Manage focus after navigation (e.g., move focus to results heading)
• Consider adding ARIA live regions for result count updates
• Ensure keyboard navigation works correctly with custom controls

💡 Suggestions

1. Add TypeScript Types to Code Examples

File: src/stories/usage-examples/SpaNavigation.mdx Throughout

Most code examples show function signatures but could benefit from explicit type imports and annotations. This would help developers understand the expected function signatures and catch type errors early.

2. Consider Adding a Troubleshooting Section

File: src/stories/usage-examples/SpaNavigation.mdx End of document

Suggestion: Add a "Common Issues" or "Troubleshooting" section covering:

• "PLP doesn't refetch after URL change" → Check that component is remounting
• "Browser back button doesn't work" → Ensure router is handling popstate correctly
• "URLs are too long" → Use the counter approach instead of URL-as-key

This would reduce support burden and help developers debug issues independently.

3. Add Performance Considerations

File: src/stories/usage-examples/SpaNavigation.mdx Line: L24 or new section

Suggestion: Briefly mention performance implications:

• Remounting on every URL change may be expensive for complex PLPs
• Consider using useMemo for expensive computations
• The library's default behavior of full page reload might actually be faster in some cases

This sets proper expectations about the trade-offs of SPA navigation.

4. Clarify Absolute URL Requirement More Prominently

File: src/stories/usage-examples/SpaNavigation.mdx Line: L278

The absolute URL requirement is mentioned in "Practical Guidance" but could be highlighted earlier. Add a note or warning box immediately after the first code example explaining that getUrl() must return an absolute URL because the library uses new URL(url) internally.

5. Example Naming Clarity

File: src/stories/usage-examples/SpaNavigation.mdx Lines: L38, L103

The examples are labeled "Option A" and "Option B" but could have more descriptive names like:

• "Option A: React Router v6 (Recommended for React Router users)"
• "Option B: Native History API (For custom routers or non-React Router SPAs)"

This makes it clearer when to use each approach.

Overall Assessment: ✅ Pass

This is excellent documentation that solves a real pain point for SPA users. The code examples are functional and well-explained. The important issues identified are primarily about consistency, edge case handling, and accessibility—all of which would improve the robustness of the guide but don't prevent it from being useful as-is. The suggestions focus on making the documentation even more helpful for developers implementing these patterns in production.