refactor(course): Isolate lesson content types through router orchestration

openspec/changes/lesson-detail-refactor/.openspec.yaml

@@ -0,0 +1,2 @@
+schema: spec-driven
+created: 2026-03-31

openspec/changes/lesson-detail-refactor/design.md

@@ -0,0 +1,28 @@
+## Context
+
+Currently, the lesson detail implementation is split between multiple screens, each with its own boilerplate for data management. We want to move to a "Domain Partitioning" model where the Router decides which specialized screen to show, instead of relying on a "Shell" widget that delegates internally.
+
+## Goals / Non-Goals
+
+**Goals:**
+
+- **Isolate PDF content**: Move PDF-specific logic to `PdfLessonDetailScreen`.
+- **Router-First Architecture**: Use `app_router.dart` as the switcher to avoid redundant widget layers.
+- **Isolate UI components**: Maintain specialized headers and layouts within each content screen for tailored UX.
+- **Standardize filenames**: Correct naming drift (`_screen.dart` vs `_page.dart`).
+
+**Non-Goals:**
+
+- **Adding new content types**: We are only refactoring existing PDF, Video, and Test paths.
+- **Cross-package UI sharing**: Each domain package (`courses`, `exams`) will maintain its own specialized detail UI.
+
+## Decisions
+
+- **Decision 1: Use the Router as the Brain**: Instead of a "Switcher" screen, we'll map paths like `/lesson/:id` directly to specialized screens in the `app_router.dart` builder.
+- **Decision 2: Standardize on `_screen.dart`**: To match your naming convention, all detail entry-point filenames will end with `_screen.dart`.
+- **Decision 3: Keep UI Isolated**: PDF and Video screens will keep their own specialized headers and sidebar logic to follow the "Independent Domain" principle.
+
+## Risks / Trade-offs
+
+- **Risk: Duplicate Data Loading Logic** → **Mitigation**: Use a common `lessonDetailProvider` inside the Router Builder so that fetching, loading, and error states are handled once for all types in a single location.
+- **Risk: Breaking Router Paths** → **Mitigation**: Update all barrel file exports (`courses.dart`) and route definitions during the deletion of the legacy shell screen.

openspec/changes/lesson-detail-refactor/proposal.md

@@ -0,0 +1,25 @@
+## Why
+
+The current lesson detail implementation is fragmented. The `LessonDetailScreen` was overloaded with PDF-specific logic while containing placeholders for video, whereas `VideoLessonDetailScreen` exists separately with its own header/footer implementation. We need a clean way to route between different lesson content types (PDF, Video, Test, Assessment) while maintaining consistent data retrieval logic without adding unnecessary "switcher" UI layers.
+
+## What Changes
+
+- **Router Orchestration**: Move content-switching logic from the widget layer into the `app_router.dart`. The router will now act as the single brain for identifying lesson types and pushing to the correct specialized screen.
+- **Isolation**: Extract all PDF-specific rendering from the legacy `LessonDetailScreen` into a dedicated `PdfLessonDetailScreen`.
+- **Standardization**: Rename `video_lesson_detail_page.dart` to `video_lesson_detail_screen.dart` for naming consistency.
+- **Redundancy Removal**: **Delete** the redundant `LessonDetailScreen` entirely to follow the "Direct Destination Screen" pattern.
+
+## Capabilities
+
+### New Capabilities
+- `lesson-router-orchestration`: A unified router-based strategy to identify lesson types (PDF, Video, etc.) and delegate rendering directly to specialized screens while sharing common data providers.
+
+### Modified Capabilities
+- `lesson-pdf-playback`: Update to reflect that PDF playback now lives in its own isolated `PdfLessonDetailScreen`.
+
+## Impact
+
+- `packages/courses/lib/screens/lesson_detail_screen.dart`: **Deleted** as redundant.
+- `packages/courses/lib/screens/pdf_lesson_detail_screen.dart`: New file for isolated PDF rendering.
+- `packages/courses/lib/screens/video_lesson_detail_screen.dart`: Standardized naming and isolated custom header.
+- `packages/testpress/lib/navigation/app_router.dart`: Updated to map `/lesson/:id` directly to `PdfLessonDetailScreen`.

openspec/changes/lesson-detail-refactor/specs/lesson-content-orchestration/spec.md

@@ -0,0 +1,19 @@
+## ADDED Requirements
+
+### Requirement: Router-Level Lesson Orchestration
+The system SHALL provide a central orchestration layer within the `app_router.dart` that identifies lesson types and delegates rendering to specialized screens (PDF, Video, Test, Assessment).
+
+#### Scenario: Routing to specialized PDF reader
+- **WHEN** a lesson of type "pdf" is navigated to via the router
+- **THEN** it SHALL be mapped directly to the `PdfLessonDetailScreen`.
+
+#### Scenario: Routing to specialized Video player
+- **WHEN** a lesson of type "video" is navigated to via the router
+- **THEN** it SHALL be mapped directly to the `VideoLessonDetailScreen`.
+
+### Requirement: Decoupled Data Loading
+The system SHALL handle the fetching of lesson domain models via `lessonDetailProvider` at the route builder level, ensuring specialized screens only receive the finished `Lesson` object.
+
+#### Scenario: Unified loading indicator across types
+- **WHEN** a lesson payload is being fetched for any screen
+- **THEN** the router SHALL display a unified `AppLoadingIndicator`.

openspec/changes/lesson-detail-refactor/specs/lesson-pdf-playback/spec.md

@@ -0,0 +1,9 @@
+## MODIFIED Requirements
+
+### Requirement: Remote PDF Rendering
+The system SHALL support rendering PDF lessons from a remote URL via a dedicated `PdfLessonDetailScreen`. The system MUST provide a full-screen viewer which supports zooming and basic navigation for the PDF content, isolated from other content types.
+
+#### Scenario: Rendering PDF content in isolated screen
+- **WHEN** a lesson of type "pdf" is opened via the router-orchestrated `PdfLessonDetailScreen`
+- **AND** the `contentUrl` is provided
+- **THEN** the system SHALL initialize the `PdfLessonDetailScreen` and display the document in the specialized viewer.

openspec/changes/lesson-detail-refactor/tasks.md

@@ -0,0 +1,22 @@
+## 1. Standardize Naming
+
+- [x] 1.1 Rename `video_lesson_detail_page.dart` to `video_lesson_detail_screen.dart`.
+- [x] 1.2 Update imports and router definitions in `packages/testpress/lib/navigation/app_router.dart` for the renamed video screen.
+
+## 2. Isolate PDF Content
+
+- [x] 2.1 Create `pdf_lesson_detail_screen.dart` with state management for PDF rendering and reading progress.
+- [x] 2.2 Move PDF-specific logic (AppPdfViewer) to the new screen.
+- [x] 2.3 Ensure PDF screen handles its own subject-specific theme and custom header.
+
+## 3. Implement Router-First Orchestration
+
+- [x] 3.1 Update `app_router.dart` builders to map `/lesson/:id` directly to `PdfLessonDetailScreen`.
+- [x] 3.2 Update `app_router.dart` to share the `lessonDetailProvider` loading logic across all lesson types.
+- [x] 3.3 Revert shared UI components: ensure `VideoLessonDetailScreen` keeps its own custom header.
+- [x] 3.4 **Delete** the redundant `LessonDetailScreen` switcher entirely.
+
+## 4. Verification & Testing
+
+- [x] 4.1 Verify that `/study/lesson/thermo-1` triggers the new `PdfLessonDetailScreen` directly.
+- [x] 4.2 Verify that `/study/video/thermo-2` triggers the refactored `VideoLessonDetailScreen` directly.

packages/courses/lib/courses.dart

@@ -27,8 +27,8 @@ export 'providers/course_detail_provider.dart';
 export 'screens/chapter_detail_page.dart';
 export 'widgets/chapter_status_filter_bar.dart';
 export 'widgets/chapter_content_item.dart';
-export 'screens/lesson_detail_screen.dart';
-export 'screens/video_lesson_detail_page.dart';
+export 'screens/pdf_lesson_detail_screen.dart';
+export 'screens/video_lesson_detail_screen.dart';
 export 'providers/lesson_detail_provider.dart';
 export 'providers/course_list_provider.dart';
 export 'providers/lesson_providers.dart';

packages/courses/lib/screens/lesson_detail_screen.dart → packages/courses/lib/screens/pdf_lesson_detail_screen.dart

@@ -9,18 +9,16 @@ import '../widgets/lesson_detail/lesson_navigation_footer.dart';
 import '../widgets/lesson_detail/pdf_viewer.dart';
 import '../providers/lesson_detail_provider.dart';
 
-/// Fullscreen reader for text-based lessons.
-///
-/// Supports rich content rendering, reading progress tracking, and navigation.
-class LessonDetailScreen extends ConsumerStatefulWidget {
-  const LessonDetailScreen({
+/// Specialized reader for PDF-based lessons.
+class PdfLessonDetailScreen extends ConsumerStatefulWidget {
+  const PdfLessonDetailScreen({
     super.key,
     required this.lesson,
     this.onNext,
     this.onPrevious,
   });
 
-  /// The lesson domain model to render.
+  /// The PDF lesson to render.
   final Lesson lesson;
 
   /// Optional callback to navigate to the next lesson.
@@ -30,62 +28,15 @@ class LessonDetailScreen extends ConsumerStatefulWidget {
   final VoidCallback? onPrevious;
 
   @override
-  ConsumerState<LessonDetailScreen> createState() => _LessonDetailScreenState();
+  ConsumerState<PdfLessonDetailScreen> createState() =>
+      _PdfLessonDetailScreenState();
 }
 
-class _LessonDetailScreenState extends ConsumerState<LessonDetailScreen> {
-  final ScrollController _scrollController = ScrollController();
+class _PdfLessonDetailScreenState extends ConsumerState<PdfLessonDetailScreen> {
   double _readingProgress = 0.0;
   bool _showDownloadFeedback = false;
   bool _alreadyMarkedComplete = false;
 
-  @override
-  void initState() {
-    super.initState();
-    _scrollController.addListener(_onScroll);
-  }
-
-  @override
-  void dispose() {
-    _scrollController.removeListener(_onScroll);
-    _scrollController.dispose();
-    super.dispose();
-  }
-
-  /// Calculates reading progress based on scroll position.
-  void _onScroll() {
-    if (!_scrollController.hasClients) return;
-
-    final maxScroll = _scrollController.position.maxScrollExtent;
-    final currentScroll = _scrollController.offset;
-
-    // Avoid division by zero if content is shorter than the viewport
-    if (maxScroll <= 0) {
-      if (_readingProgress != 1.0) {
-        setState(() => _readingProgress = 1.0);
-        if (widget.lesson.progressStatus != LessonProgressStatus.completed) {
-          _markAsCompleted();
-        }
-      }
-      return;
-    }
-
-    final newProgress = (currentScroll / maxScroll).clamp(0.0, 1.0);
-    if ((newProgress - _readingProgress).abs() > 0.01) {
-      setState(() {
-        _readingProgress = newProgress;
-      });
-
-      // Mark as completed if scroll exceeds 99%
-      if (newProgress >= 0.99 &&
-          !_alreadyMarkedComplete &&
-          widget.lesson.progressStatus != LessonProgressStatus.completed) {
-        _alreadyMarkedComplete = true;
-        _markAsCompleted();
-      }
-    }
-  }
-
   Future<void> _markAsCompleted() async {
     final repository = await ref.read(courseRepositoryProvider.future);
     await repository.updateLessonProgress(
@@ -134,13 +85,38 @@ class _LessonDetailScreenState extends ConsumerState<LessonDetailScreen> {
               LessonReadingProgressBar(
                 progress: _readingProgress,
                 foregroundColor: subjectColors?.accent,
-                animationDuration: widget.lesson.type == LessonType.pdf
-                    ? const Duration(milliseconds: 50)
-                    : const Duration(milliseconds: 300),
+                animationDuration: const Duration(milliseconds: 50),
               ),
-              // Main Content Area
+              // Main PDF Content Area
               Expanded(
-                child: _buildMediaContent(context, design, subjectColors),
+                child: Column(
+                  children: [
+                    Expanded(
+                      child: widget.lesson.contentUrl != null
+                          ? AppPdfViewer(
+                              url: widget.lesson.contentUrl!,
+                              onProgressChanged: (progress) {
+                                setState(() => _readingProgress = progress);
+
+                                if (progress >= 0.99 &&
+                                    !_alreadyMarkedComplete &&
+                                    widget.lesson.progressStatus !=
+                                        LessonProgressStatus.completed) {
+                                  _alreadyMarkedComplete = true;
+                                  _markAsCompleted();
+                                }
+                              },
+                            )
+                          : Center(
+                              child: AppText.body(
+                                L10n.of(context).chapterNoContent,
+                                color: design.colors.textSecondary,
+                              ),
+                            ),
+                    ),
+                    _buildNavigationFooter(design),
+                  ],
+                ),
               ),
             ],
           ),
@@ -156,49 +132,6 @@ class _LessonDetailScreenState extends ConsumerState<LessonDetailScreen> {
     );
   }
 
-  Widget _buildMediaContent(
-    BuildContext context,
-    DesignConfig design,
-    dynamic subjectColors,
-  ) {
-    // PDF Lesson: Render the new premium PDF viewer
-    if (widget.lesson.type == LessonType.pdf &&
-        widget.lesson.contentUrl != null) {
-      return Column(
-        children: [
-          Expanded(
-            child: AppPdfViewer(
-              url: widget.lesson.contentUrl!,
-              onProgressChanged: (progress) {
-                setState(() => _readingProgress = progress);
-
-                // Mark as completed if progress exceeds 99%
-                if (progress >= 0.99 &&
-                    !_alreadyMarkedComplete &&
-                    widget.lesson.progressStatus !=
-                        LessonProgressStatus.completed) {
-                  _alreadyMarkedComplete = true;
-                  _markAsCompleted();
-                }
-              },
-            ),
-          ),
-          _buildNavigationFooter(design),
-        ],
-      );
-    }
-
-    // Video Lesson: For now, fallback or empty (will be consolidated later or handled by VideoDetailScreen)
-    if (widget.lesson.type == LessonType.video) {
-      return const Center(child: AppText.body('Video content coming soon...'));
-    }
-
-    // Fallback: Empty state or assessment placeholder
-    return Center(
-      child: AppText.body('No content available for this lesson type'),
-    );
-  }
-
   Widget _buildNavigationFooter(DesignConfig design) {
     return Padding(
       padding: EdgeInsets.all(design.spacing.md),
@@ -212,8 +145,6 @@ class _LessonDetailScreenState extends ConsumerState<LessonDetailScreen> {
   }
 }
 
-
-
 class _CustomToast extends StatelessWidget {
   const _CustomToast({required this.message});
   final String message;

packages/testpress/lib/navigation/app_router.dart

@@ -226,7 +226,7 @@ final goRouterProvider = Provider<GoRouter>((ref) {
                   builder: (context, state) {
                     final lessonArg = state.extra as Lesson?;
                     if (lessonArg != null) {
-                      return LessonDetailScreen(lesson: lessonArg);
+                      return PdfLessonDetailScreen(lesson: lessonArg);
                     }
 
                     final id = state.pathParameters['id']!;
@@ -240,7 +240,7 @@ final goRouterProvider = Provider<GoRouter>((ref) {
                                 child: Text('Lesson not found'),
                               );
                             }
-                            return LessonDetailScreen(lesson: lesson);
+                            return PdfLessonDetailScreen(lesson: lesson);
                           },
                           loading: () => Container(
                             color: const Color(0xFFFFFFFF),