[FEATURE] [MER-4057] Remixing/Customize Content Updates (PR2: Frontend Polish)

[Francisco-Castro]

Summary

PR2 of MER-4057 — Frontend polish for the Template Customize Content (remix) page. Builds on PR1 (#6416) which added the backend container creation, scope isolation, and functional UI.

What changed

Phase 4: Add Materials Modal

• Migrated from deprecated OliWeb.Common.Modal (Bootstrap) to OliWeb.Components.Modal
• Preselected items (already in curriculum) are hidden, not disabled — curriculum tab filters in-memory, All Pages tab filters at SQL level via exclude_resource_ids
• Added subtitle: "Materials can only be added to the curriculum once."
• Buttons use design system Button component (primary/secondary)
• Modal styling matches Figma (rounded corners, border, shadow, padding, close button)

Phase 5: Unsaved Changes Modal + Saving Feedback

• New UnsavedChangesModal component with warning icon, "Leave Without Saving" / "Save Changes" buttons
• NavigationGuard JS hook intercepts all link clicks (capture phase) when unsaved changes exist — shows modal instead of navigating
• Modifier keys (Ctrl/Cmd+click, Shift+click, middle-click) bypass the guard for new-tab behavior
• beforeunload fallback for hard navigation (tab close, refresh)
• Save stays on page (per designer) — shows "Your work has been saved." flash banner, reloads state
• Error flash on save failure
• Cancel resets hierarchy to previous state (stays on page)
• Navigation target validated server-side (rejects external URLs)

Phase 6: Design System Updates

• Buttons: Add Materials (+icon), Create Unit, Cancel, Save, Move, Hide, Remove — all migrated to Button component
• Action buttons: arrow_circle_right and trash_filled icons from Figma SVGs, !px-4 padding
• Cancel/Save disabled state: muted colors matching Figma (transparent bg, muted border for secondary; white text on light blue for primary)
• Cursor: !cursor-default on disabled buttons (no "not-permitted" icon)
• Title: Bold 24px/32px with text-Text-text-high token
• Description: Medium 16px/24px, added "Updates will not affect previously created course sections, only new ones."
• Spacing: 6px gap title→description, 24px description→buttons
• Tabs: design system tokens, 2px active indicator, border-Fill-Buttons-fill-primary
• Breadcrumbs: home icon (home_filled), white bg with border on nav bar
• Checkboxes: styled with border-Border-border-default, bg-Surface-surface-background
• Workspace breadcrumbs: new style (text links + chevron separator), kept in layout header per designer

Design System Fixes (cross-cutting)

• Button secondary bg: bg-Background-bg-primary → bg-Surface-surface-background
• Button disabled primary: white text on muted blue (was dark text, no shadow)
• Button disabled secondary: transparent bg, muted border/text (was grey)
• fill-primary token: #0062F2 → #0080FF (synced with Figma)
• Added Text-text-medium token (#737373)
• Added icons: home_filled, arrow_circle_right, trash_filled, warning_triangle
• Added container_class, title_class, subtitle_class attrs to Modal component
• Breadcrumb bar px-3 padding removed from workspace layout

Security

• NavigationGuard validates navigation targets server-side (valid_internal_path?/1)
• Rejects external URLs from forged LiveView events

Performance

• Preselected exclusion IDs cached in modal_assigns (computed once per publication selection)
• Removed redundant Sections.get_section! DB call on save

Screen.Recording.2026-04-16.at.10.34.39.AM.mov

[github-actions]

	Warnings
⚠️	PR is large (1169 LOC changed). Consider splitting.

Note: risk rules not loaded — micromatch is not a function

Generated by 🚫 dangerJS against ff86cd5

[github-actions]

AI Review — performance

Large NOT IN exclusion list can degrade query planning

file: lib/oli/publishing.ex
line: 746
Description: The new rev.resource_id not in ^ids predicate pushes the full exclude_resource_ids list into SQL. When this list is large, PostgreSQL must handle a large parameter set for each request, which can slow planning/execution on the hierarchy picker query path.
Suggestion: Replace the large NOT IN list with an anti-join approach (for example, join against unnest(^ids) and filter is_nil on the joined key), or otherwise bound/externalize exclusions so the DB does not repeatedly evaluate large inline lists.

Rebuilding MapSet during render path adds avoidable allocation cost

file: lib/oli_web/live/common/hierarchy/hierarchy_picker.ex
line: 425
Description: preselected_set = MapSet.new(preselected) is rebuilt inside reject_preselected/2, which runs in the component render flow. With frequent LiveView updates and larger preselected lists, this creates repeated O(n) allocations and extra GC pressure.
Suggestion: Precompute and store preselected_set once when modal assigns are built/updated, then pass it through assigns and use it directly in reject_preselected/2.

Save handlers perform synchronous full state reload in LiveView event loop

file: lib/oli_web/live/delivery/remix_section.ex
line: 473
Description: After save, handlers call reload_remix_state/2 (Remix.init_open_and_free/1) inline before responding. If this call performs substantial DB/workspace reconstruction, it blocks the LiveView process and increases interaction latency.
Suggestion: Keep save handlers fast by avoiding full reload in the hot path (update only required assigns), or run reload asynchronously (assign_async/start_async) and render once data is ready. For navigation-after-save flows, navigate immediately after successful save.

[github-actions]

AI Review — security

No issues found

[github-actions]

AI Review — elixir

Save Action In Unsaved-Changes Flow Does Not Continue Navigation

file: lib/oli_web/live/delivery/remix_section.ex
line: 473
Description: In "unsaved_changes_save", a successful save only updates assigns/flash and closes modal state, but does not navigate to the previously requested target. This contradicts the modal flow (“leave without saving” vs “save changes”) and can trap users on the page after choosing to proceed.
Suggestion: After successful save/reload, read pending_navigation_target (fallback as needed) and push_navigate/2 to that path, then clear modal/pending state.

LiveView Can Crash On Reload Failure Due To Hard Match

file: lib/oli_web/live/delivery/remix_section.ex
line: 431
Description: {:ok, new_state} = reload_remix_state(section, socket) will raise if reload returns {:error, reason}. In an event handler this can crash the LiveView process instead of returning controlled UI feedback.
Suggestion: Replace hard match with with/case and handle {:error, reason} by setting an error flash and preserving current state.

Preselected Guard Removed From Table Rows, Enabling Duplicate Selection If Caller Misses Filtering

file: lib/oli_web/live/common/hierarchy/hierarchy_picker_table_model.ex
line: 48
Description: The diff removes preselected-based click/disabled behavior and makes every row selectable. This weakens component-level safety and allows already-added materials to be selected again if any caller omits exclude_resource_ids filtering.
Suggestion: Keep a defensive UI guard in render_child/2 (disable checkbox + remove click handler when {pub.id, child.revision.resource_id} is preselected), even if query-level filtering is also applied.

[github-actions]

AI Review — ui

Save/Cancel appear disabled but remain actionable

file: lib/oli_web/live/delivery/remix_section.html.heex
line: 29
Description: The buttons use !cursor-default when @has_unsaved_changes is false, but they are still fully clickable/keyboard-activatable. This creates misleading affordance and fails expected disabled-state behavior.
Suggestion: Set an actual disabled state on both buttons (for example disabled={!@has_unsaved_changes}), and ensure disabled styling/aria-disabled is applied by the button component.

Decorative breadcrumb chevron is exposed to assistive tech

file: lib/oli_web/live/breadcrumb/breadcrumb_trail_workspace_live.ex
line: 30
Description: The separator chevron is decorative but rendered as a normal icon inside a list item, so screen readers may announce extra non-informative content between breadcrumbs.
Suggestion: Mark the separator as decorative (for example aria-hidden="true" on the separator element/icon, and optionally focusable="false" on the SVG).

Add Materials modal close target is too small for touch accessibility

file: lib/oli_web/live/delivery/remix/add_materials_modal.ex
line: 49
Description: The close control uses size-5 (20x20), which is below recommended minimum touch target size. This makes close actions harder on touch devices.
Suggestion: Increase the button hit area to at least 24px (preferably ~44px) while keeping icon size unchanged (for example larger padding/container dimensions).

Unsaved Changes modal close target is too small for touch accessibility

file: lib/oli_web/live/delivery/remix/unsaved_changes_modal.ex
line: 35
Description: The close control is also size-5 (20x20), below recommended touch target guidance, reducing usability on mobile/touch.
Suggestion: Increase the interactive button area to at least 24px (preferably ~44px), with icon centered inside.

[github-actions]

PrivSignal Risk: HIGH

Scoring guide

Top 10 contributing items

1. [R-HIGH-EXTERNAL-FLOW-ADDED] edge_added (high, flow confidence: 0.5) for lti_registration_client_id (+8 related refs) at lib/oli_web/controllers/lti_controller.ex:802
2. [R-HIGH-EXTERNAL-FLOW-ADDED] edge_added (high, flow confidence: 0.5) for lti_registration_issuer (+8 related refs) at lib/oli_web/controllers/lti_controller.ex:802
3. [R-HIGH-EXTERNAL-FLOW-ADDED] edge_added (high, flow confidence: 0.5) for lti_params_deployment_id (+8 related refs) at lib/oli_web/controllers/lti_controller.ex:749
4. [R-HIGH-EXTERNAL-FLOW-ADDED] edge_added (high, flow confidence: 0.5) for institution_pending_registration_issuer (+8 related refs) at lib/oli_web/controllers/lti_controller.ex:749
5. [R-HIGH-EXTERNAL-FLOW-ADDED] edge_added (high, flow confidence: 0.7) for conversation_message_name (+10 related refs) at lib/oli_web/controllers/api/lti_controller.ex:138
6. [R-HIGH-EXTERNAL-FLOW-ADDED] edge_added (high, flow confidence: 0.7) for institution_pending_registration_client_id (+10 related refs) at lib/oli_web/controllers/api/lti_controller.ex:97
7. [R-HIGH-EXTERNAL-FLOW-ADDED] edge_added (high, flow confidence: 0.5) for institution_pending_registration_client_id (+8 related refs) at lib/oli_web/controllers/lti_controller.ex:802
8. [R-HIGH-EXTERNAL-FLOW-ADDED] edge_added (high, flow confidence: 0.7) for user_name (+10 related refs) at lib/oli_web/controllers/api/lti_controller.ex:97
9. [R-HIGH-EXTERNAL-FLOW-ADDED] edge_added (high, flow confidence: 0.7) for cookies_consent_name (+10 related refs) at lib/oli_web/controllers/api/lti_controller.ex:138
10. [R-HIGH-EXTERNAL-FLOW-ADDED] edge_added (high, flow confidence: 0.7) for author_name (+10 related refs) at lib/oli_web/controllers/api/lti_controller.ex:97

[github-actions]

AI Review — typescript

Empty href bypasses unsaved-changes guard

file: assets/src/hooks/navigation_guard.ts
line: 53
Description: The guard returns early when href is an empty string (!href), but <a href=""> is a valid navigation that reloads/navigates to the current page. In dirty state, that path can bypass the modal flow and lose unsaved changes.
Suggestion: Treat empty href as a navigable internal target instead of skipping it, e.g. resolve via new URL(href, window.location.href) and guard it like other same-origin links.