Courses panel refresh + narrow-screen fixes (#7692, #7697, #7700, #7707, #7685)#7711
Conversation
When the participant permissions action is opened from a course (a Space), the popup row label and the permission-chooser dialog title now read "Course permissions" instead of "Chat permissions". Mirrors the existing isSpace branching already used for ban/unban in the same menu. Adds a coursePermissions l10n key and threads an isCourse flag into showPermissionChooser (defaulting false, so other callers are unchanged). Closes #7707 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MSQX5Y55ycHjcZkaJhc15W
On narrow screens the activity-map search clear (X) button did nothing: the bar is built by the shell but its onQueryChanged reaches only the map's State via a GlobalKey, so clearing the query rebuilds the map subtree but never the shell-built bar. Its didUpdateWidget never fires, so the field's controller and the X-button visibility (both keyed off the stale external query) never update. Drive the bar off its own TextEditingController instead: clear it locally on press, and compute the "searching" state from the controller's text so the button tracks the field as the user types, backspaces and clears. Closes #7685 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MSQX5Y55ycHjcZkaJhc15W
A quest-plan whose learning_objective_sequence is empty has no content to build a course from — it surfaced as a selectable "0 modules" card that the learner can't actually turn into a course. Drop these rows in the quest-plans -> CoursePlanModel adapter, using the same null-to-filter contract as the existing missing-field guards, so they never reach the creation picker (or the repo's other consumers: find-course, the course-plan provider, onboarding). Closes #7700 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MSQX5Y55ycHjcZkaJhc15W
…full course Courses panel content (single- and two-column): - With at least one joined course, the three add-course actions (start my own / enter a code / browse public) move to the panel header as compact right-justified icons, so the joined-course list keeps the vertical space; with no courses they stay as full-width buttons in the body (the empty state). Both hub render-sites are centralized into one CoursesHubPanel. Courses sheet default height (narrow): - The Courses hub opens content-fit — tall enough to show all joined courses (or the add buttons when there are none), capped at the available height — instead of defaulting to half. Closes #7692. Nav widget / map search bar (narrow): - Once a course sheet is pulled to full it covers the map, so the floating map search bar hides entirely and its reserved strip is handed to the course content; the bar returns when the sheet drops below full. The nav widget reports a latched full-height state up to the shell, stable across drags so the reservation (and the drag coordinate space) doesn't thrash. Closes #7697. Updates routing.instructions.md for the content-fit hub, the header actions, and the full-course search-bar hide. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MSQX5Y55ycHjcZkaJhc15W
📝 WalkthroughWalkthroughThe PR reorganizes Courses hub rendering and add-course actions, reports settled mobile cavity height, hides search at full course-sheet height, fixes mobile search clearing, distinguishes course permissions, rejects empty quest plans, and updates routing documentation. ChangesCourse hub and add-course flow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant MobileNavWidget
participant WorkspaceShell
participant CoursesHubPanel
participant MobileSearchBar
User->>CoursesHubPanel: open Courses hub
CoursesHubPanel-->>WorkspaceShell: provide course content
WorkspaceShell->>MobileNavWidget: set preferred cavity height
MobileNavWidget-->>WorkspaceShell: report settled full-height state
WorkspaceShell->>MobileSearchBar: hide at full height
User->>MobileNavWidget: drag below full height
MobileNavWidget-->>WorkspaceShell: report not full
WorkspaceShell->>MobileSearchBar: restore compact search button
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
lib/features/quests/repo/quest_plans_repo.dart (1)
128-133: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd regression coverage for empty quest plans.
Cover both
learning_objective_sequence: []and a missing sequence returningnull, plus a non-empty sequence producing the expected placeholder count. Place the test undertest/pangea/and run it withfvm flutter test.As per path instructions, Pangea tests belong under
test/pangea/and must use the pinned Flutter command.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/features/quests/repo/quest_plans_repo.dart` around lines 128 - 133, Add regression tests under test/pangea/ for the quest-plan conversion/filtering behavior around the missionCount guard: verify an empty learning_objective_sequence returns null, a missing sequence also returns null, and a non-empty sequence produces the expected placeholder mission count. Run the added test with the pinned fvm flutter test command.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/routes/world/left_panel/left_panel_courses_list_view.dart`:
- Around line 52-73: Convert CoursesHubPanel to a StatefulWidget and cache the
filtered, rate-limited client.onSync stream in its State.didChangeDependencies
method. Recreate the cached stream only when the client changes, and have build
use that stable stream instead of constructing the where/rateLimit pipeline
inline.
---
Nitpick comments:
In `@lib/features/quests/repo/quest_plans_repo.dart`:
- Around line 128-133: Add regression tests under test/pangea/ for the
quest-plan conversion/filtering behavior around the missionCount guard: verify
an empty learning_objective_sequence returns null, a missing sequence also
returns null, and a non-empty sequence produces the expected placeholder mission
count. Run the added test with the pinned fvm flutter test command.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 0ec92a32-94ab-49f8-a1d1-ffdeae3dde26
📒 Files selected for processing (13)
.github/instructions/routing.instructions.mdlib/features/quests/repo/quest_plans_repo.dartlib/l10n/intl_en.arblib/routes/courses/add_course_options.dartlib/routes/world/left_panel/left_panel_add_course_subpage.dartlib/routes/world/left_panel/left_panel_courses_list_view.dartlib/routes/world/left_panel/workspace_left_panel.dartlib/routes/world/mobile_search_bar.dartlib/widgets/layouts/mobile_nav_widget.dartlib/widgets/layouts/workspace_shell.dartlib/widgets/permission_slider_dialog.dartlib/widgets/users/member_actions_popup_menu_button.darttest/pangea/navigation/mobile_nav_widget_test.dart
| return StreamBuilder( | ||
| stream: client.onSync.stream | ||
| .where((s) => s.hasRoomUpdate) | ||
| .rateLimit(const Duration(seconds: 1)), | ||
| builder: (context, _) { | ||
| final courses = | ||
| client.rooms | ||
| .where( | ||
| (r) => | ||
| r.isSpace && | ||
| r.membership == Membership.join && | ||
| r.coursePlan != null, | ||
| ) | ||
| .toList() | ||
| ..sort( | ||
| (a, b) => a | ||
| .getLocalizedDisplayname(MatrixLocals(l10n)) | ||
| .toLowerCase() | ||
| .compareTo( | ||
| b | ||
| .getLocalizedDisplayname(MatrixLocals(l10n)) | ||
| .toLowerCase(), | ||
| ), | ||
| ); | ||
|
|
||
| return ListView( | ||
| padding: const EdgeInsets.fromLTRB(12.0, 4.0, 12.0, 16.0), | ||
| final courses = joinedCourses(client, l10n); | ||
| return Column( | ||
| children: [ | ||
| for (final space in courses) CourseListTile(space), | ||
| // #7172: a user in no courses isn't a course-without-a-plan, so don't | ||
| // show the "this course needs a plan" message here — the "Add new | ||
| // course" section below is the empty state and invites them to join | ||
| // or create one. | ||
| const SizedBox(height: 4.0), | ||
| // "Add new course" section divider + the add options. | ||
| Row( | ||
| children: [ | ||
| Expanded( | ||
| child: Divider(color: theme.colorScheme.outlineVariant), | ||
| ), | ||
| Padding( | ||
| padding: const EdgeInsets.symmetric(horizontal: 12.0), | ||
| child: Text( | ||
| l10n.addNewCourse, | ||
| style: theme.textTheme.labelLarge?.copyWith( | ||
| color: theme.colorScheme.onSurfaceVariant, | ||
| ), | ||
| ), | ||
| ), | ||
| Expanded( | ||
| child: Divider(color: theme.colorScheme.outlineVariant), | ||
| ), | ||
| ], | ||
| PanelHeader( | ||
| leading: closeButton, | ||
| title: l10n.courses, | ||
| // With courses present, the three add-course actions ride the | ||
| // header as right-justified icons; when empty they stay as full | ||
| // buttons in the body below (the empty state). | ||
| trailing: | ||
| courses.isEmpty ? null : const AddCourseHeaderActions(), | ||
| ), | ||
| const SizedBox(height: 12.0), | ||
| const AddCourseOptions(), | ||
| Expanded(child: LeftPanelCoursesListView(courses: courses)), | ||
| ], | ||
| ); | ||
| }, | ||
| ); |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
Cache the derived stream to prevent subscription churn.
Creating a new stream pipeline (.where(...).rateLimit(...)) inside build() causes StreamBuilder to unsubscribe and resubscribe every time the widget rebuilds (e.g., when inherited dependencies like L10n or Matrix update). This defeats the rate-limiter and causes unnecessary memory churn.
Consider converting CoursesHubPanel to a StatefulWidget and caching the stream in didChangeDependencies so it is only recreated when the client actually changes.
🛠 Proposed fix
-class CoursesHubPanel extends StatelessWidget {
- final Widget closeButton;
-
- const CoursesHubPanel({super.key, required this.closeButton});
-
- `@override`
- Widget build(BuildContext context) {
- final client = Matrix.of(context).client;
- final l10n = L10n.of(context);
-
- return StreamBuilder(
- stream: client.onSync.stream
- .where((s) => s.hasRoomUpdate)
- .rateLimit(const Duration(seconds: 1)),
- builder: (context, _) {
- final courses = joinedCourses(client, l10n);
- return Column(
- children: [
- PanelHeader(
- leading: closeButton,
- title: l10n.courses,
- // With courses present, the three add-course actions ride the
- // header as right-justified icons; when empty they stay as full
- // buttons in the body below (the empty state).
- trailing:
- courses.isEmpty ? null : const AddCourseHeaderActions(),
- ),
- Expanded(child: LeftPanelCoursesListView(courses: courses)),
- ],
- );
- },
- );
- }
-}
+class CoursesHubPanel extends StatefulWidget {
+ final Widget closeButton;
+
+ const CoursesHubPanel({super.key, required this.closeButton});
+
+ `@override`
+ State<CoursesHubPanel> createState() => _CoursesHubPanelState();
+}
+
+class _CoursesHubPanelState extends State<CoursesHubPanel> {
+ Client? _client;
+ late Stream<dynamic> _roomUpdateStream;
+
+ `@override`
+ void didChangeDependencies() {
+ super.didChangeDependencies();
+ final newClient = Matrix.of(context).client;
+ if (newClient != _client) {
+ _client = newClient;
+ _roomUpdateStream = _client!.onSync.stream
+ .where((s) => s.hasRoomUpdate)
+ .rateLimit(const Duration(seconds: 1));
+ }
+ }
+
+ `@override`
+ Widget build(BuildContext context) {
+ final l10n = L10n.of(context);
+
+ return StreamBuilder(
+ stream: _roomUpdateStream,
+ builder: (context, _) {
+ final courses = joinedCourses(_client!, l10n);
+ return Column(
+ children: [
+ PanelHeader(
+ leading: widget.closeButton,
+ title: l10n.courses,
+ trailing:
+ courses.isEmpty ? null : const AddCourseHeaderActions(),
+ ),
+ Expanded(child: LeftPanelCoursesListView(courses: courses)),
+ ],
+ );
+ },
+ );
+ }
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return StreamBuilder( | |
| stream: client.onSync.stream | |
| .where((s) => s.hasRoomUpdate) | |
| .rateLimit(const Duration(seconds: 1)), | |
| builder: (context, _) { | |
| final courses = | |
| client.rooms | |
| .where( | |
| (r) => | |
| r.isSpace && | |
| r.membership == Membership.join && | |
| r.coursePlan != null, | |
| ) | |
| .toList() | |
| ..sort( | |
| (a, b) => a | |
| .getLocalizedDisplayname(MatrixLocals(l10n)) | |
| .toLowerCase() | |
| .compareTo( | |
| b | |
| .getLocalizedDisplayname(MatrixLocals(l10n)) | |
| .toLowerCase(), | |
| ), | |
| ); | |
| return ListView( | |
| padding: const EdgeInsets.fromLTRB(12.0, 4.0, 12.0, 16.0), | |
| final courses = joinedCourses(client, l10n); | |
| return Column( | |
| children: [ | |
| for (final space in courses) CourseListTile(space), | |
| // #7172: a user in no courses isn't a course-without-a-plan, so don't | |
| // show the "this course needs a plan" message here — the "Add new | |
| // course" section below is the empty state and invites them to join | |
| // or create one. | |
| const SizedBox(height: 4.0), | |
| // "Add new course" section divider + the add options. | |
| Row( | |
| children: [ | |
| Expanded( | |
| child: Divider(color: theme.colorScheme.outlineVariant), | |
| ), | |
| Padding( | |
| padding: const EdgeInsets.symmetric(horizontal: 12.0), | |
| child: Text( | |
| l10n.addNewCourse, | |
| style: theme.textTheme.labelLarge?.copyWith( | |
| color: theme.colorScheme.onSurfaceVariant, | |
| ), | |
| ), | |
| ), | |
| Expanded( | |
| child: Divider(color: theme.colorScheme.outlineVariant), | |
| ), | |
| ], | |
| PanelHeader( | |
| leading: closeButton, | |
| title: l10n.courses, | |
| // With courses present, the three add-course actions ride the | |
| // header as right-justified icons; when empty they stay as full | |
| // buttons in the body below (the empty state). | |
| trailing: | |
| courses.isEmpty ? null : const AddCourseHeaderActions(), | |
| ), | |
| const SizedBox(height: 12.0), | |
| const AddCourseOptions(), | |
| Expanded(child: LeftPanelCoursesListView(courses: courses)), | |
| ], | |
| ); | |
| }, | |
| ); | |
| class CoursesHubPanel extends StatefulWidget { | |
| final Widget closeButton; | |
| const CoursesHubPanel({super.key, required this.closeButton}); | |
| `@override` | |
| State<CoursesHubPanel> createState() => _CoursesHubPanelState(); | |
| } | |
| class _CoursesHubPanelState extends State<CoursesHubPanel> { | |
| Client? _client; | |
| late Stream<dynamic> _roomUpdateStream; | |
| `@override` | |
| void didChangeDependencies() { | |
| super.didChangeDependencies(); | |
| final newClient = Matrix.of(context).client; | |
| if (newClient != _client) { | |
| _client = newClient; | |
| _roomUpdateStream = _client!.onSync.stream | |
| .where((s) => s.hasRoomUpdate) | |
| .rateLimit(const Duration(seconds: 1)); | |
| } | |
| } | |
| `@override` | |
| Widget build(BuildContext context) { | |
| final l10n = L10n.of(context); | |
| return StreamBuilder( | |
| stream: _roomUpdateStream, | |
| builder: (context, _) { | |
| final courses = joinedCourses(_client!, l10n); | |
| return Column( | |
| children: [ | |
| PanelHeader( | |
| leading: widget.closeButton, | |
| title: l10n.courses, | |
| trailing: | |
| courses.isEmpty ? null : const AddCourseHeaderActions(), | |
| ), | |
| Expanded(child: LeftPanelCoursesListView(courses: courses)), | |
| ], | |
| ); | |
| }, | |
| ); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/routes/world/left_panel/left_panel_courses_list_view.dart` around lines
52 - 73, Convert CoursesHubPanel to a StatefulWidget and cache the filtered,
rate-limited client.onSync stream in its State.didChangeDependencies method.
Recreate the cached stream only when the client changes, and have build use that
stable stream instead of constructing the where/rateLimit pipeline inline.
Summary
A batch of GitHub-issue fixes plus a courses-panel refresh, developed together on this branch.
Courses panel
CoursesHubPanel.World map / nav (narrow screens)
Course creation
Permissions
isSpacewording.routing.instructions.mdis updated for the content-fit hub, the header actions, and the full-course search-bar hide.Pull Request has been tested on:
Not run in this environment — see the note above. Needs a QA pass on at least one narrow-screen target.
Accessibility
tooltip:names (reusing the existing add-course labels); no new decorative images. See accessibility.instructions.md.🤖 Generated with Claude Code
https://claude.ai/code/session_01MSQX5Y55ycHjcZkaJhc15W
Generated by Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation