From 36b62dd0e1ea7790553d3e6dc12b1ebe45e1c079 Mon Sep 17 00:00:00 2001 From: syed-tp Date: Fri, 11 Sep 2026 16:18:54 +0530 Subject: [PATCH 1/3] feat: optimize chapter tab filtering by removing skeleton flicker and auto-disposing filter state --- .../.openspec.yaml | 2 + .../optimize-chapter-tab-filtering/design.md | 18 +++ .../proposal.md | 22 +++ .../specs/chapter-tab-filtering/spec.md | 15 ++ .../optimize-chapter-tab-filtering/tasks.md | 6 + .../lib/screens/chapter_detail_page.dart | 2 +- .../widgets/chapter_status_filter_bar.dart | 3 +- .../screens/chapter_detail_page_test.dart | 153 ++++++++++++++++++ 8 files changed, 219 insertions(+), 2 deletions(-) create mode 100644 openspec/changes/optimize-chapter-tab-filtering/.openspec.yaml create mode 100644 openspec/changes/optimize-chapter-tab-filtering/design.md create mode 100644 openspec/changes/optimize-chapter-tab-filtering/proposal.md create mode 100644 openspec/changes/optimize-chapter-tab-filtering/specs/chapter-tab-filtering/spec.md create mode 100644 openspec/changes/optimize-chapter-tab-filtering/tasks.md create mode 100644 packages/courses/test/screens/chapter_detail_page_test.dart diff --git a/openspec/changes/optimize-chapter-tab-filtering/.openspec.yaml b/openspec/changes/optimize-chapter-tab-filtering/.openspec.yaml new file mode 100644 index 000000000..d3e5afff4 --- /dev/null +++ b/openspec/changes/optimize-chapter-tab-filtering/.openspec.yaml @@ -0,0 +1,2 @@ +schema_version: 1 +name: optimize-chapter-tab-filtering diff --git a/openspec/changes/optimize-chapter-tab-filtering/design.md b/openspec/changes/optimize-chapter-tab-filtering/design.md new file mode 100644 index 000000000..8af151fd4 --- /dev/null +++ b/openspec/changes/optimize-chapter-tab-filtering/design.md @@ -0,0 +1,18 @@ +# Design: Optimize Chapter Tab Filtering UX + +## Context +In `ChapterDetailPage`, content status filtering allows users to toggle between `All`, `Running`, `Upcoming`, and `History`. While `initialSync` is updating lesson status flags in the background, filtering currently displays a skeleton shimmer whenever `filteredLessons.isEmpty`. + +## Technical Strategy + +1. **Rendering Condition in `ChapterDetailPage`**: + - Change `isSyncing && filteredLessons.isEmpty` to `isSyncing && chapter.lessons.isEmpty`. + - When `chapter.lessons.isNotEmpty`, the list directly renders `filteredLessons`. If `filteredLessons.isEmpty`, render `l10n.chapterNoContent` immediately. + - When background status sync finishes, Drift/Riverpod stream emits updated `LessonDto`s with new status flags, seamlessly repopulating the filtered list without UI blocking. + +2. **Provider Scope in `chapter_status_filter_bar.dart`**: + - Update `chapterStatusFilterProvider` to `StateProvider.autoDispose`. + - Ensures filter selection resets upon exiting/navigating across chapters. + +3. **Testing**: + - Add widget tests covering tab switching without skeleton flicker and verifying filter reset on navigation. diff --git a/openspec/changes/optimize-chapter-tab-filtering/proposal.md b/openspec/changes/optimize-chapter-tab-filtering/proposal.md new file mode 100644 index 000000000..436e1c41e --- /dev/null +++ b/openspec/changes/optimize-chapter-tab-filtering/proposal.md @@ -0,0 +1,22 @@ +## Why + +When switching between status filter tabs (All, Running, Upcoming, History) in the chapter detail screen, users and testers may observe a 1–2 second skeleton shimmer "delay" if background status synchronization is in flight. + +This happens because `ChapterDetailPage` checks `if (isSyncing && filteredLessons.isEmpty)` to render 5 skeleton cards. If a chapter already has local/cached lessons but none match the selected tab (or before background status flags update), the UI replaces content with skeleton placeholders before snapping back to the real list or empty state. Additionally, `chapterStatusFilterProvider` is not auto-disposed, which can cause tab filter selections to persist across different chapters. + +## What Changes + +- Decouple skeleton loader rendering from tab-filtered empty states: only show skeleton shimmer if the entire chapter has no lessons loaded (`isSyncing && chapter.lessons.isEmpty`). +- Render empty states immediately when `chapter.lessons.isNotEmpty` but `filteredLessons.isEmpty`. +- Make `chapterStatusFilterProvider` auto-disposed (`StateProvider.autoDispose`) so each chapter navigation resets the active filter to `All`. +- Add unit/widget tests to verify tab filtering and auto-dispose behavior. + +## Capabilities + +### Modified Capabilities +- `lms-study-chapter-detail`: Update tab filtering behavior to eliminate skeleton flicker when switching tabs on loaded chapters and ensure filter state resets between chapter visits. + +## Impact +- `packages/courses/lib/screens/chapter_detail_page.dart` +- `packages/courses/lib/widgets/chapter_status_filter_bar.dart` +- `packages/courses/test/screens/chapter_detail_page_test.dart` diff --git a/openspec/changes/optimize-chapter-tab-filtering/specs/chapter-tab-filtering/spec.md b/openspec/changes/optimize-chapter-tab-filtering/specs/chapter-tab-filtering/spec.md new file mode 100644 index 000000000..29f286b5b --- /dev/null +++ b/openspec/changes/optimize-chapter-tab-filtering/specs/chapter-tab-filtering/spec.md @@ -0,0 +1,15 @@ +# Spec: Chapter Tab Filtering + +## Requirements + +### Requirement: Instant Tab Filtering on Available Curriculum +The system SHALL filter chapter lessons immediately in memory when switching between `All`, `Running`, `Upcoming`, and `History` tabs. + +#### Scenario: Switching tab when chapter lessons exist +- **GIVEN** a chapter with loaded lessons +- **WHEN** the user switches between status filter tabs while background synchronization is in progress +- **THEN** the system SHALL NOT display skeleton shimmer loaders +- **AND** the system SHALL display the filtered lessons matching the active tab or the empty state immediately. + +### Requirement: Filter State Scope +The system SHALL reset the chapter status filter to `All` when leaving or entering a chapter detail screen. diff --git a/openspec/changes/optimize-chapter-tab-filtering/tasks.md b/openspec/changes/optimize-chapter-tab-filtering/tasks.md new file mode 100644 index 000000000..130417315 --- /dev/null +++ b/openspec/changes/optimize-chapter-tab-filtering/tasks.md @@ -0,0 +1,6 @@ +# Implementation Tasks + +- [x] 1. Update `chapterStatusFilterProvider` in `packages/courses/lib/widgets/chapter_status_filter_bar.dart` to use `StateProvider.autoDispose`. +- [x] 2. Update `ChapterDetailPage` in `packages/courses/lib/screens/chapter_detail_page.dart` to only show skeleton loaders when `chapter.lessons.isEmpty` during syncing. +- [x] 3. Create widget tests in `packages/courses/test/screens/chapter_detail_page_test.dart` to verify tab switching and autoDispose behavior. +- [x] 4. Run tests and verify all tests pass. diff --git a/packages/courses/lib/screens/chapter_detail_page.dart b/packages/courses/lib/screens/chapter_detail_page.dart index 435164b42..de252d609 100644 --- a/packages/courses/lib/screens/chapter_detail_page.dart +++ b/packages/courses/lib/screens/chapter_detail_page.dart @@ -128,7 +128,7 @@ class _ChapterDetailPageState extends ConsumerState { vertical: design.spacing.md, ), children: [ - if (isSyncing && filteredLessons.isEmpty) + if (isSyncing && chapter.lessons.isEmpty) ..._skeletonLessons.map( (lesson) => ChapterContentItem( lesson: lesson, diff --git a/packages/courses/lib/widgets/chapter_status_filter_bar.dart b/packages/courses/lib/widgets/chapter_status_filter_bar.dart index 6644c00c1..0aeec1c04 100644 --- a/packages/courses/lib/widgets/chapter_status_filter_bar.dart +++ b/packages/courses/lib/widgets/chapter_status_filter_bar.dart @@ -6,7 +6,8 @@ import 'package:flutter_riverpod/flutter_riverpod.dart'; enum ChapterStatusFilter { all, running, upcoming, history } /// State provider for the active chapter status filter. -final chapterStatusFilterProvider = StateProvider( +final chapterStatusFilterProvider = + StateProvider.autoDispose( (ref) => ChapterStatusFilter.all, ); diff --git a/packages/courses/test/screens/chapter_detail_page_test.dart b/packages/courses/test/screens/chapter_detail_page_test.dart new file mode 100644 index 000000000..8334c4392 --- /dev/null +++ b/packages/courses/test/screens/chapter_detail_page_test.dart @@ -0,0 +1,153 @@ +import 'package:core/core.dart'; +import 'package:flutter/widgets.dart'; +import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:courses/courses.dart'; +import 'package:courses/providers/chapter_detail_provider.dart'; + +void main() { + final testLessons = [ + const LessonDto( + id: 'lesson-1', + chapterId: 'chapter-1', + title: 'Running Lesson', + type: LessonType.video, + progressStatus: LessonProgressStatus.notStarted, + orderIndex: 0, + duration: '00:10:00', + isLocked: false, + isRunning: true, + isUpcoming: false, + hasAttempts: false, + ), + const LessonDto( + id: 'lesson-2', + chapterId: 'chapter-1', + title: 'Upcoming Lesson', + type: LessonType.video, + progressStatus: LessonProgressStatus.notStarted, + orderIndex: 1, + duration: '00:15:00', + isLocked: false, + isRunning: false, + isUpcoming: true, + hasAttempts: false, + ), + ]; + + final testChapter = ChapterDto( + id: 'chapter-1', + courseId: 'course-1', + title: 'Chapter 1 Title', + lessonCount: 2, + assessmentCount: 0, + orderIndex: 1, + isLeaf: true, + lessons: testLessons, + ); + + Widget wrap(Widget child, {List overrides = const []}) { + return ProviderScope( + overrides: overrides, + child: DesignProvider( + config: DesignConfig.defaults(), + child: LocalizationProvider( + child: Builder( + builder: (context) { + final locale = LocalizationProvider.of(context).locale; + return Localizations( + locale: locale, + delegates: LocalizationProvider.delegates, + child: Directionality( + textDirection: TextDirection.ltr, + child: child, + ), + ); + }, + ), + ), + ), + ); + } + + group('ChapterDetailPage tab switching and skeleton tests', () { + testWidgets( + 'switching tabs when lessons exist does NOT show skeleton loaders', + (tester) async { + await tester.pumpWidget( + wrap( + const ChapterDetailPage( + courseId: 'course-1', + chapterId: 'chapter-1', + ), + overrides: [ + chapterDetailProvider('course-1', 'chapter-1').overrideWith( + (ref) => Stream.value((testChapter, 'Test Course'))), + chapterDetailControllerProvider + .overrideWith(() => _MockSyncingController()), + ], + ), + ); + + await tester.pumpAndSettle(); + + // Initial state: 'All' filter shows both lessons + expect(find.text('Running Lesson'), findsOneWidget); + expect(find.text('Upcoming Lesson'), findsOneWidget); + + // Tap 'Running' filter tab + await tester.tap(find.text('Running')); + await tester.pumpAndSettle(); + + // Should show only Running Lesson immediately, no skeleton + expect(find.text('Running Lesson'), findsOneWidget); + expect(find.text('Upcoming Lesson'), findsNothing); + expect(find.text('Loading lesson title text content'), findsNothing); + + // Tap 'Upcoming' filter tab + await tester.tap(find.text('Upcoming')); + await tester.pumpAndSettle(); + + // Should show only Upcoming Lesson immediately, no skeleton + expect(find.text('Upcoming Lesson'), findsOneWidget); + expect(find.text('Running Lesson'), findsNothing); + expect(find.text('Loading lesson title text content'), findsNothing); + + // Tap 'History' filter tab (empty) + await tester.tap(find.text('History')); + await tester.pumpAndSettle(); + + // Should show empty state without any skeleton loaders + expect(find.text('Loading lesson title text content'), findsNothing); + expect(find.text('No content available'), findsOneWidget); + }); + + test( + 'chapterStatusFilterProvider resets to default when container disposed', + () { + final container = ProviderContainer(); + final sub = container.listen(chapterStatusFilterProvider, (_, __) {}); + + expect( + container.read(chapterStatusFilterProvider), ChapterStatusFilter.all); + + container.read(chapterStatusFilterProvider.notifier).state = + ChapterStatusFilter.upcoming; + expect(container.read(chapterStatusFilterProvider), + ChapterStatusFilter.upcoming); + + sub.close(); + container.dispose(); + + final freshContainer = ProviderContainer(); + expect(freshContainer.read(chapterStatusFilterProvider), + ChapterStatusFilter.all); + freshContainer.dispose(); + }); + }); +} + +class _MockSyncingController extends ChapterDetailController { + @override + bool build() => true; // Simulate background sync in progress +} From 0ac2fc2f0f16f8dd9c6aee1942c061dba1ce09a7 Mon Sep 17 00:00:00 2001 From: syed-tp Date: Fri, 11 Sep 2026 16:28:41 +0530 Subject: [PATCH 2/3] test: add integration test to verify chapter status filter resets on page navigation --- .../screens/chapter_detail_page_test.dart | 81 +++++++++++++++++-- 1 file changed, 74 insertions(+), 7 deletions(-) diff --git a/packages/courses/test/screens/chapter_detail_page_test.dart b/packages/courses/test/screens/chapter_detail_page_test.dart index 8334c4392..f3f688b10 100644 --- a/packages/courses/test/screens/chapter_detail_page_test.dart +++ b/packages/courses/test/screens/chapter_detail_page_test.dart @@ -4,6 +4,7 @@ import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:courses/courses.dart'; import 'package:courses/providers/chapter_detail_provider.dart'; +import 'package:courses/widgets/chapter_status_filter_bar.dart'; void main() { final testLessons = [ @@ -122,9 +123,74 @@ void main() { expect(find.text('No content available'), findsOneWidget); }); + testWidgets( + 'leaving and re-entering ChapterDetailPage resets filter to All', + (tester) async { + final host = ValueNotifier('chapter-1'); + + await tester.pumpWidget( + wrap( + ValueListenableBuilder( + valueListenable: host, + builder: (context, chapterId, _) { + if (chapterId == null) { + return const SizedBox.shrink(); + } + return ChapterDetailPage( + key: ValueKey(chapterId), + courseId: 'course-1', + chapterId: chapterId, + ); + }, + ), + overrides: [ + chapterDetailProvider('course-1', 'chapter-1').overrideWith( + (ref) => Stream.value((testChapter, 'Test Course'))), + chapterDetailProvider('course-1', 'chapter-2') + .overrideWith((ref) => Stream.value(( + testChapter.copyWith( + id: 'chapter-2', + title: 'Chapter 2 Title', + ), + 'Test Course', + ))), + chapterDetailControllerProvider + .overrideWith(() => _MockSyncingController()), + ], + ), + ); + + await tester.pumpAndSettle(); + + // Initial state shows both lessons + expect(find.text('Running Lesson'), findsOneWidget); + expect(find.text('Upcoming Lesson'), findsOneWidget); + + // Select 'Upcoming' tab + await tester.tap(find.text('Upcoming')); + await tester.pumpAndSettle(); + + // Only upcoming lesson is shown + expect(find.text('Upcoming Lesson'), findsOneWidget); + expect(find.text('Running Lesson'), findsNothing); + + // Navigate away (unmount ChapterDetailPage) + host.value = null; + await tester.pumpAndSettle(); + + // Navigate to Chapter 2 + host.value = 'chapter-2'; + await tester.pumpAndSettle(); + + // On Chapter 2, autoDispose reset filter to 'All', showing both lessons + expect(find.textContaining('Chapter 2 Title'), findsOneWidget); + expect(find.text('Running Lesson'), findsOneWidget); + expect(find.text('Upcoming Lesson'), findsOneWidget); + }); + test( - 'chapterStatusFilterProvider resets to default when container disposed', - () { + 'chapterStatusFilterProvider auto-disposes and resets on the same container', + () async { final container = ProviderContainer(); final sub = container.listen(chapterStatusFilterProvider, (_, __) {}); @@ -137,12 +203,13 @@ void main() { ChapterStatusFilter.upcoming); sub.close(); - container.dispose(); + // Allow the autoDispose microtask scheduled by Riverpod to execute + await Future(() {}); - final freshContainer = ProviderContainer(); - expect(freshContainer.read(chapterStatusFilterProvider), - ChapterStatusFilter.all); - freshContainer.dispose(); + // Reading again on the same container must return the initial state 'all' + expect( + container.read(chapterStatusFilterProvider), ChapterStatusFilter.all); + container.dispose(); }); }); } From c625e9dac41df9dcc767099e3683020e052bce7e Mon Sep 17 00:00:00 2001 From: syed-tp Date: Fri, 11 Sep 2026 16:57:10 +0530 Subject: [PATCH 3/3] refactor: remove unused chapter_status_filter_bar import from chapter_detail_page_test --- packages/courses/test/screens/chapter_detail_page_test.dart | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/courses/test/screens/chapter_detail_page_test.dart b/packages/courses/test/screens/chapter_detail_page_test.dart index f3f688b10..500ac3f2d 100644 --- a/packages/courses/test/screens/chapter_detail_page_test.dart +++ b/packages/courses/test/screens/chapter_detail_page_test.dart @@ -4,7 +4,6 @@ import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:courses/courses.dart'; import 'package:courses/providers/chapter_detail_provider.dart'; -import 'package:courses/widgets/chapter_status_filter_bar.dart'; void main() { final testLessons = [