From 250c4962541645c3825e9a7b8f3fefa221b87eb6 Mon Sep 17 00:00:00 2001 From: syed-tp Date: Tue, 31 Mar 2026 15:28:26 +0530 Subject: [PATCH 1/2] refactor: replace redundant LessonDetailScreen with router-level orchestration and specialized content screens --- .../lesson-detail-refactor/.openspec.yaml | 2 + .../changes/lesson-detail-refactor/design.md | 28 ++++ .../lesson-detail-refactor/proposal.md | 25 ++++ .../lesson-content-orchestration/spec.md | 19 +++ .../specs/lesson-pdf-playback/spec.md | 9 ++ .../changes/lesson-detail-refactor/tasks.md | 22 +++ packages/courses/lib/courses.dart | 4 +- ...een.dart => pdf_lesson_detail_screen.dart} | 136 ++++-------------- ...e.dart => video_lesson_detail_screen.dart} | 0 .../testpress/lib/navigation/app_router.dart | 4 +- 10 files changed, 139 insertions(+), 110 deletions(-) create mode 100644 openspec/changes/lesson-detail-refactor/.openspec.yaml create mode 100644 openspec/changes/lesson-detail-refactor/design.md create mode 100644 openspec/changes/lesson-detail-refactor/proposal.md create mode 100644 openspec/changes/lesson-detail-refactor/specs/lesson-content-orchestration/spec.md create mode 100644 openspec/changes/lesson-detail-refactor/specs/lesson-pdf-playback/spec.md create mode 100644 openspec/changes/lesson-detail-refactor/tasks.md rename packages/courses/lib/screens/{lesson_detail_screen.dart => pdf_lesson_detail_screen.dart} (54%) rename packages/courses/lib/screens/{video_lesson_detail_page.dart => video_lesson_detail_screen.dart} (100%) diff --git a/openspec/changes/lesson-detail-refactor/.openspec.yaml b/openspec/changes/lesson-detail-refactor/.openspec.yaml new file mode 100644 index 00000000..8fb86310 --- /dev/null +++ b/openspec/changes/lesson-detail-refactor/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-03-31 diff --git a/openspec/changes/lesson-detail-refactor/design.md b/openspec/changes/lesson-detail-refactor/design.md new file mode 100644 index 00000000..7ea6c5f0 --- /dev/null +++ b/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. diff --git a/openspec/changes/lesson-detail-refactor/proposal.md b/openspec/changes/lesson-detail-refactor/proposal.md new file mode 100644 index 00000000..4ed8bd9e --- /dev/null +++ b/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`. diff --git a/openspec/changes/lesson-detail-refactor/specs/lesson-content-orchestration/spec.md b/openspec/changes/lesson-detail-refactor/specs/lesson-content-orchestration/spec.md new file mode 100644 index 00000000..c1be0c01 --- /dev/null +++ b/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`. diff --git a/openspec/changes/lesson-detail-refactor/specs/lesson-pdf-playback/spec.md b/openspec/changes/lesson-detail-refactor/specs/lesson-pdf-playback/spec.md new file mode 100644 index 00000000..121c20b8 --- /dev/null +++ b/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 unified `LessonDetailScreen` +- **AND** the `contentUrl` is provided +- **THEN** the system SHALL initialize the `PdfLessonDetailScreen` and display the document in the specialized viewer. diff --git a/openspec/changes/lesson-detail-refactor/tasks.md b/openspec/changes/lesson-detail-refactor/tasks.md new file mode 100644 index 00000000..9dbb377d --- /dev/null +++ b/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. diff --git a/packages/courses/lib/courses.dart b/packages/courses/lib/courses.dart index dbe80656..a7bab156 100644 --- a/packages/courses/lib/courses.dart +++ b/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'; diff --git a/packages/courses/lib/screens/lesson_detail_screen.dart b/packages/courses/lib/screens/pdf_lesson_detail_screen.dart similarity index 54% rename from packages/courses/lib/screens/lesson_detail_screen.dart rename to packages/courses/lib/screens/pdf_lesson_detail_screen.dart index 4c3b20b9..0fdd9bb5 100644 --- a/packages/courses/lib/screens/lesson_detail_screen.dart +++ b/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 createState() => _LessonDetailScreenState(); + ConsumerState createState() => + _PdfLessonDetailScreenState(); } -class _LessonDetailScreenState extends ConsumerState { - final ScrollController _scrollController = ScrollController(); +class _PdfLessonDetailScreenState extends ConsumerState { 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 _markAsCompleted() async { final repository = await ref.read(courseRepositoryProvider.future); await repository.updateLessonProgress( @@ -134,13 +85,31 @@ class _LessonDetailScreenState extends ConsumerState { 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: AppPdfViewer( + url: widget.lesson.contentUrl ?? '', + onProgressChanged: (progress) { + setState(() => _readingProgress = progress); + + if (progress >= 0.99 && + !_alreadyMarkedComplete && + widget.lesson.progressStatus != + LessonProgressStatus.completed) { + _alreadyMarkedComplete = true; + _markAsCompleted(); + } + }, + ), + ), + _buildNavigationFooter(design), + ], + ), ), ], ), @@ -156,49 +125,6 @@ class _LessonDetailScreenState extends ConsumerState { ); } - 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 +138,6 @@ class _LessonDetailScreenState extends ConsumerState { } } - - class _CustomToast extends StatelessWidget { const _CustomToast({required this.message}); final String message; diff --git a/packages/courses/lib/screens/video_lesson_detail_page.dart b/packages/courses/lib/screens/video_lesson_detail_screen.dart similarity index 100% rename from packages/courses/lib/screens/video_lesson_detail_page.dart rename to packages/courses/lib/screens/video_lesson_detail_screen.dart diff --git a/packages/testpress/lib/navigation/app_router.dart b/packages/testpress/lib/navigation/app_router.dart index 8b74e3e1..d7c8280c 100644 --- a/packages/testpress/lib/navigation/app_router.dart +++ b/packages/testpress/lib/navigation/app_router.dart @@ -226,7 +226,7 @@ final goRouterProvider = Provider((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((ref) { child: Text('Lesson not found'), ); } - return LessonDetailScreen(lesson: lesson); + return PdfLessonDetailScreen(lesson: lesson); }, loading: () => Container( color: const Color(0xFFFFFFFF), From 18ff4b4caa5500edb34141051ab11ed9996c00a4 Mon Sep 17 00:00:00 2001 From: syed-tp Date: Tue, 31 Mar 2026 16:26:38 +0530 Subject: [PATCH 2/2] feat: add empty state to PdfLessonDetailScreen and update specification documentation --- .../specs/lesson-pdf-playback/spec.md | 2 +- .../lib/screens/pdf_lesson_detail_screen.dart | 33 +++++++++++-------- 2 files changed, 21 insertions(+), 14 deletions(-) diff --git a/openspec/changes/lesson-detail-refactor/specs/lesson-pdf-playback/spec.md b/openspec/changes/lesson-detail-refactor/specs/lesson-pdf-playback/spec.md index 121c20b8..d2f1ba87 100644 --- a/openspec/changes/lesson-detail-refactor/specs/lesson-pdf-playback/spec.md +++ b/openspec/changes/lesson-detail-refactor/specs/lesson-pdf-playback/spec.md @@ -4,6 +4,6 @@ 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 unified `LessonDetailScreen` +- **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. diff --git a/packages/courses/lib/screens/pdf_lesson_detail_screen.dart b/packages/courses/lib/screens/pdf_lesson_detail_screen.dart index 0fdd9bb5..7f3a5cdc 100644 --- a/packages/courses/lib/screens/pdf_lesson_detail_screen.dart +++ b/packages/courses/lib/screens/pdf_lesson_detail_screen.dart @@ -92,20 +92,27 @@ class _PdfLessonDetailScreenState extends ConsumerState { child: Column( children: [ Expanded( - child: AppPdfViewer( - url: widget.lesson.contentUrl ?? '', - onProgressChanged: (progress) { - setState(() => _readingProgress = progress); + 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(); - } - }, - ), + 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), ],