Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
schema_version: 1
name: optimize-chapter-tab-filtering
18 changes: 18 additions & 0 deletions openspec/changes/optimize-chapter-tab-filtering/design.md
Original file line number Diff line number Diff line change
@@ -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<ChapterStatusFilter>`.
- 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.
22 changes: 22 additions & 0 deletions openspec/changes/optimize-chapter-tab-filtering/proposal.md
Original file line number Diff line number Diff line change
@@ -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`
Original file line number Diff line number Diff line change
@@ -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.
6 changes: 6 additions & 0 deletions openspec/changes/optimize-chapter-tab-filtering/tasks.md
Original file line number Diff line number Diff line change
@@ -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.
2 changes: 1 addition & 1 deletion packages/courses/lib/screens/chapter_detail_page.dart
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ class _ChapterDetailPageState extends ConsumerState<ChapterDetailPage> {
vertical: design.spacing.md,
),
children: [
if (isSyncing && filteredLessons.isEmpty)
if (isSyncing && chapter.lessons.isEmpty)
..._skeletonLessons.map(
(lesson) => ChapterContentItem(
lesson: lesson,
Expand Down
3 changes: 2 additions & 1 deletion packages/courses/lib/widgets/chapter_status_filter_bar.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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<ChapterStatusFilter>(
final chapterStatusFilterProvider =
StateProvider.autoDispose<ChapterStatusFilter>(
(ref) => ChapterStatusFilter.all,
);

Expand Down
219 changes: 219 additions & 0 deletions packages/courses/test/screens/chapter_detail_page_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,219 @@
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<Override> 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);
});

testWidgets(
'leaving and re-entering ChapterDetailPage resets filter to All',
(tester) async {
final host = ValueNotifier<String?>('chapter-1');

await tester.pumpWidget(
wrap(
ValueListenableBuilder<String?>(
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 auto-disposes and resets on the same container',
() async {
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();
// Allow the autoDispose microtask scheduled by Riverpod to execute
await Future(() {});

// Reading again on the same container must return the initial state 'all'
expect(
container.read(chapterStatusFilterProvider), ChapterStatusFilter.all);
container.dispose();
});
});
}

class _MockSyncingController extends ChapterDetailController {
@override
bool build() => true; // Simulate background sync in progress
}