feat: updates to activity start page UI - #6779
Merged
Merged
Conversation
5 tasks
Collaborator
Author
|
@avashilling @wcjord Hey these are the updates to the activity start page based on Ava's designs in the attached ticket. I'm not going to merge them yet since they're not completely tested. If you would like to pick them up while I'm away, please feel free! |
transparent gradient and name of role above avatar
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthrough
ChangesActivity Session UI Refactor: Role Goals & Subpage Navigation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
lib/pangea/activity_sessions/activity_session_start/select_role_session_controller.dart (1)
84-101: 💤 Low valueConsider caching completed goal IDs if room counts grow.
This getter iterates through all Matrix rooms synchronously each time it's accessed. Since it's called during the build path (via
ActivitySessionButtons), this could cause UI jank if the user has many rooms. For typical use cases this is likely acceptable, but if room counts become large, consider caching the result and invalidating on room/role changes.🤖 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/pangea/activity_sessions/activity_session_start/select_role_session_controller.dart` around lines 84 - 101, The selectedRoleCompletedGoalIds getter synchronously iterates through all Matrix rooms each time it's accessed, which could cause performance issues with many rooms since it's called during the build path. Implement caching for this computation by storing the result in a private field and providing an invalidation mechanism. Invalidate the cache when the selected role ID changes (in the setter for _selectedRoleId) and when the Matrix room list changes. Update the getter to return the cached value if valid, or recompute and cache it if the cache is invalid.lib/pangea/activity_sessions/activity_session_start/activity_session_button_widget.dart (1)
28-35: 💤 Low valueConsider extracting the cast to avoid redundant type checks.
The
sessionController is SelectRoleSessionControllercheck and cast is performed twice. This could be simplified:♻️ Suggested refactor
- final goals = sessionController is SelectRoleSessionController - ? (sessionController as SelectRoleSessionController).selectedRoleGoals - : null; - - final completedGoalIds = sessionController is SelectRoleSessionController - ? (sessionController as SelectRoleSessionController) - .selectedRoleCompletedGoalIds - : const <String>{}; + final selectRoleController = sessionController is SelectRoleSessionController + ? sessionController + : null; + final goals = selectRoleController?.selectedRoleGoals; + final completedGoalIds = + selectRoleController?.selectedRoleCompletedGoalIds ?? const <String>{};🤖 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/pangea/activity_sessions/activity_session_start/activity_session_button_widget.dart` around lines 28 - 35, The code performs the same type check `sessionController is SelectRoleSessionController` and cast `(sessionController as SelectRoleSessionController)` twice when assigning both the `goals` and `completedGoalIds` variables. Extract this to a single type check and cast by first assigning the cast result to an intermediate variable (e.g., `selectRoleController`) when the type check passes, then use that variable to access both `selectedRoleGoals` and `selectedRoleCompletedGoalIds`. This eliminates the redundant type checking and casting operations.
🤖 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/pangea/activity_sessions/activity_session_start/activity_sessions_start_view.dart`:
- Around line 82-283: The modified code blocks in both files are missing the
required Pangea change boundary markers as per coding guidelines. In
lib/pangea/activity_sessions/activity_session_start/activity_sessions_start_view.dart
at lines 82-283, wrap the entire modified Column widget and its children with a
// `#Pangea` comment at the start and a // Pangea# comment at the end. Similarly,
in lib/pangea/activity_sessions/activity_participant_indicator.dart at lines
89-124, wrap the modified block with the same // `#Pangea` opening and // Pangea#
closing comment markers. These boundary comments must surround all
Pangea-specific changes to clearly mark them in FluffyChat files.
- Around line 164-235: The Row widget containing the three chip Container
widgets (with CEFR level, participant count, and mode) will overflow on small
screens with longer localized text. Replace the Row widget with a Wrap widget to
allow the chips to automatically reflow to the next line when they exceed the
available width instead of overflowing. Maintain the spacing property to
preserve the 12.0 horizontal/vertical gaps between chips as they wrap.
---
Nitpick comments:
In
`@lib/pangea/activity_sessions/activity_session_start/activity_session_button_widget.dart`:
- Around line 28-35: The code performs the same type check `sessionController is
SelectRoleSessionController` and cast `(sessionController as
SelectRoleSessionController)` twice when assigning both the `goals` and
`completedGoalIds` variables. Extract this to a single type check and cast by
first assigning the cast result to an intermediate variable (e.g.,
`selectRoleController`) when the type check passes, then use that variable to
access both `selectedRoleGoals` and `selectedRoleCompletedGoalIds`. This
eliminates the redundant type checking and casting operations.
In
`@lib/pangea/activity_sessions/activity_session_start/select_role_session_controller.dart`:
- Around line 84-101: The selectedRoleCompletedGoalIds getter synchronously
iterates through all Matrix rooms each time it's accessed, which could cause
performance issues with many rooms since it's called during the build path.
Implement caching for this computation by storing the result in a private field
and providing an invalidation mechanism. Invalidate the cache when the selected
role ID changes (in the setter for _selectedRoleId) and when the Matrix room
list changes. Update the getter to return the cached value if valid, or
recompute and cache it if the cache is invalid.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f5f3ab37-993c-45bd-a160-51e9381263a9
📒 Files selected for processing (4)
lib/pangea/activity_sessions/activity_participant_indicator.dartlib/pangea/activity_sessions/activity_session_start/activity_session_button_widget.dartlib/pangea/activity_sessions/activity_session_start/activity_sessions_start_view.dartlib/pangea/activity_sessions/activity_session_start/select_role_session_controller.dart
- dropdown goals from the top, where they are in active activities - put activity description and vocab back under roles - don't let roles wrap, handles shrinking overflow
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
lib/pangea/activity_sessions/activity_session_start/not_started_session_controller.dart (1)
24-119:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMissing required Pangea modification boundary markers in changed blocks.
Please bracket the updated sections with
//#Pangea... `// Pangea#`. As per coding guidelines, `lib/**/*.dart`: Mark Pangea modifications in FluffyChat files with `// `#Pangea...// Pangea#comments.🤖 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/pangea/activity_sessions/activity_session_start/not_started_session_controller.dart` around lines 24 - 119, The changed sections in this file are missing the required Pangea modification boundary markers. Bracket the entire changed block from the NotStartedSubPage enum definition through the end of the NotStartedSessionController class with `// `#Pangea`` at the start and `// Pangea#` at the end, following the Pangea coding guidelines for marking modifications in FluffyChat files.Source: Coding guidelines
lib/pangea/activity_sessions/activity_session_start/select_role_session_controller.dart (2)
44-104:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMissing required Pangea modification boundary markers in changed blocks.
Please wrap these Pangea edits with
//#Pangea... `// Pangea#`. As per coding guidelines, `lib/**/*.dart`: Mark Pangea modifications in FluffyChat files with `// `#Pangea...// Pangea#comments.🤖 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/pangea/activity_sessions/activity_session_start/select_role_session_controller.dart` around lines 44 - 104, Wrap the Pangea modifications in the SelectRoleSessionController class (in the file lib/pangea/activity_sessions/activity_session_start/select_role_session_controller.dart) with the required boundary markers. Add `// `#Pangea`` comment before the didUpdateWidget method and `// Pangea#` comment after the showStarsCard method to denote the Pangea-modified section. Alternatively, if these changes span multiple separate modification blocks, wrap each individual modified method or property (such as didUpdateWidget, the getters for activityRoom, canConfirmRole, descriptionText, selectedRoleGoals, selectedRoleCompletedGoalIds, and the methods completedGoalIdsForRole and showStarsCard) with their own `// `#Pangea`` and `// Pangea#` markers according to your team's convention for marking FluffyChat modifications.Source: Coding guidelines
125-142:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
_confirmedis set before validation, which can leave the UI stuck in confirmed mode on early exits.Set
_confirmedonly afteractivityandselectedRoleIdare validated, and reset it on failed async flows.Proposed patch
Future<void> confirmRoleSelection() async { - setState(() => _confirmed = true); final activity = widget.activity; if (activity == null) { ErrorHandler.logError( e: "Activity plan model is null in confirmRoleSelection", data: {}, ); return; } final selectedRoleId = _selectedRoleId; if (selectedRoleId == null) { ErrorHandler.logError( e: "Cannot confirm role selection without selectedRoleId", data: {}, ); return; } + setState(() => _confirmed = true);🤖 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/pangea/activity_sessions/activity_session_start/select_role_session_controller.dart` around lines 125 - 142, The `_confirmed` state is being set to true at the start of the method before validation checks for `activity` and `selectedRoleId`. This leaves the UI in a confirmed state if either validation fails and the method returns early. Move the `setState(() => _confirmed = true);` call to after both the `activity` null check and `selectedRoleId` null check in the confirmRoleSelection method, so the UI only enters confirmed mode after validation succeeds. Additionally, ensure `_confirmed` is reset to false if any subsequent async operations fail during the role selection flow.lib/pangea/activity_sessions/activity_session_start/activity_session_button_widget.dart (1)
159-208:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMissing required Pangea boundary markers in touched FluffyChat Dart regions.
Both changed regions need explicit//#Pangea`` ...// Pangea#wrappers to comply with repository rules.
lib/pangea/activity_sessions/activity_session_start/activity_session_button_widget.dart#L159-L208: wrap the modified CTA/sub-page logic block with//#Pangea`` and// Pangea#.lib/pangea/activity_sessions/activity_session_start/activity_session_bottom_content.dart#L39-L75: wrap the modified_visibleStatuses/rendering block with//#Pangea`` and// Pangea#.As per coding guidelines, “Mark Pangea modifications in FluffyChat files with
//#Pangea`` ...// Pangea#comments”.🤖 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/pangea/activity_sessions/activity_session_start/activity_session_button_widget.dart` around lines 159 - 208, Add Pangea boundary markers to comply with repository guidelines by wrapping modified code blocks with `// `#Pangea`` and `// Pangea#` comments. In `lib/pangea/activity_sessions/activity_session_start/activity_session_button_widget.dart` at lines 159-208, wrap the modified CTA/sub-page logic block (the if/else conditional structure handling subPage, neededParticipants, and button rendering) with the opening `// `#Pangea`` marker before the if statement checking controller.subPage and closing `// Pangea#` marker after the last closing bracket. Similarly, in `lib/pangea/activity_sessions/activity_session_start/activity_session_bottom_content.dart` at lines 39-75, wrap the modified `_visibleStatuses`/rendering block with opening `// `#Pangea`` and closing `// Pangea#` markers around the affected code section.Source: Coding guidelines
🧹 Nitpick comments (2)
lib/pangea/activity_sessions/activity_participant_indicator.dart (1)
133-150: 💤 Low valueConsider adding semantic labels for accessibility.
The star icons convey goal completion status visually but lack semantics for screen readers. Adding
Semanticsor usingTooltipwould improve accessibility.♿ Suggested improvement
Wrap( alignment: WrapAlignment.center, spacing: 2.0, runSpacing: 4.0, children: goals!.map((g) { final done = completedGoalIds?.contains(g.id) ?? false; - return Icon( - done ? Icons.star : Icons.star_border, - size: 22.0, - color: done - ? (theme.brightness == Brightness.light - ? AppConfig.gold - : AppConfig.goldLight) - : null, - ); + return Semantics( + label: done ? 'Goal completed' : 'Goal not completed', + child: Icon( + done ? Icons.star : Icons.star_border, + size: 22.0, + color: done + ? (theme.brightness == Brightness.light + ? AppConfig.gold + : AppConfig.goldLight) + : null, + ), + ); }).toList(), ),🤖 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/pangea/activity_sessions/activity_participant_indicator.dart` around lines 133 - 150, The star icons in the goal completion display lack semantic labels for screen readers, making them inaccessible. Wrap each Icon widget (the one returning either Icons.star or Icons.star_border) with a Semantics widget that includes a label property describing the completion status. The label should clearly indicate whether each goal is completed or not, for example using text like "Goal completed" for the filled star icon and "Goal not completed" for the empty star icon. This will ensure screen readers can communicate the goal completion status to users with visual impairments.lib/pangea/activity_sessions/activity_session_start/activity_session_state_controller.dart (1)
35-53: ⚡ Quick win
scanCompletedGoalIdsis O(rooms) per invocation and will be called repeatedly from UI getters.This helper does a full room scan each call; when invoked per-role during rebuilds, it can become a UI-thread hot path. Consider memoizing by
(activityId, roleId)for the current sync snapshot.🤖 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/pangea/activity_sessions/activity_session_start/activity_session_state_controller.dart` around lines 35 - 53, The scanCompletedGoalIds method performs a full room scan on every invocation, which becomes a performance bottleneck when called repeatedly from UI getters during rebuilds. Add memoization to this method by caching results based on the (activityId, roleId) tuple for the current sync snapshot. Store the cached results in a field (such as a Map or similar structure) and invalidate the cache when the activity data or rooms list changes. This will prevent redundant room scans within the same sync cycle while maintaining correctness by clearing stale results when the underlying data updates.
🤖 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/pangea/activity_sessions/activity_session_start/activity_session_state_controller.dart`:
- Line 8: The `ActivitySessionStateController` is defined as a mixin but
violates the controller pattern requirement for files named `*controller.dart`
under `lib/pangea/`. Either convert `ActivitySessionStateController` to a proper
controller class that extends `ChangeNotifier` or `BaseController<T>`, or if it
must remain a mixin, rename it (remove "Controller" suffix) and move it to a
different file location outside the `*controller.dart` naming convention to
align with the coding guidelines.
- Around line 8-55: The ActivitySessionStateController mixin definition is
missing the required Pangea modification boundary markers. Wrap the entire mixin
definition (the mixin declaration with all its methods and properties including
the static scanCompletedGoalIds method) with `// `#Pangea`` at the beginning and
`// Pangea#` at the end to properly mark this as a Pangea modification in the
FluffyChat codebase, following the coding guidelines for Dart files in the lib
directory.
In
`@lib/pangea/activity_sessions/activity_session_start/activity_sessions_start_view.dart`:
- Around line 286-298: The didUpdateWidget method in _ActivityGoalsDropdownState
has two issues: the delayed callback that sets _displayGoals to null can clear
newly arrived goals causing flicker, and the code unconditionally sets
_innerExpanded to true without respecting the startCollapsed parameter. Fix the
first issue by either cancelling the delayed future if new goals arrive or by
tracking the timing to prevent older delayed operations from overwriting newer
state. Fix the second issue by changing the unconditional _innerExpanded = true
assignment to respect the startCollapsed parameter, such as by setting
_innerExpanded = !widget.startCollapsed.
In
`@lib/pangea/activity_sessions/activity_session_start/not_started_session_controller.dart`:
- Around line 48-50: The NotStartedSessionController class incorrectly extends
State<NotStartedSession> instead of following the required controller pattern.
Change the class declaration to extend ChangeNotifier or BaseController<T> as
mandated by the coding guidelines for controller files in
lib/pangea/**/*controller.dart. Remove the State extension and mixin reference,
and implement the appropriate controller base class that provides the necessary
state management contract.
In
`@lib/pangea/activity_sessions/activity_session_start/select_role_session_controller.dart`:
- Around line 43-44: The SelectRoleSessionController class violates the
controller contract by extending State<SelectRoleSession> instead of
ChangeNotifier or BaseController<T>. Refactor SelectRoleSessionController to
extend either ChangeNotifier or BaseController<T> (depending on your project's
preference for change notification versus stream-based state management), and
ensure the ActivitySessionStateController mixin is compatible with the chosen
base class. This will align the controller with the coding guidelines for
lib/pangea/**/*controller.dart files.
---
Outside diff comments:
In
`@lib/pangea/activity_sessions/activity_session_start/activity_session_button_widget.dart`:
- Around line 159-208: Add Pangea boundary markers to comply with repository
guidelines by wrapping modified code blocks with `// `#Pangea`` and `// Pangea#`
comments. In
`lib/pangea/activity_sessions/activity_session_start/activity_session_button_widget.dart`
at lines 159-208, wrap the modified CTA/sub-page logic block (the if/else
conditional structure handling subPage, neededParticipants, and button
rendering) with the opening `// `#Pangea`` marker before the if statement checking
controller.subPage and closing `// Pangea#` marker after the last closing
bracket. Similarly, in
`lib/pangea/activity_sessions/activity_session_start/activity_session_bottom_content.dart`
at lines 39-75, wrap the modified `_visibleStatuses`/rendering block with
opening `// `#Pangea`` and closing `// Pangea#` markers around the affected code
section.
In
`@lib/pangea/activity_sessions/activity_session_start/not_started_session_controller.dart`:
- Around line 24-119: The changed sections in this file are missing the required
Pangea modification boundary markers. Bracket the entire changed block from the
NotStartedSubPage enum definition through the end of the
NotStartedSessionController class with `// `#Pangea`` at the start and `//
Pangea#` at the end, following the Pangea coding guidelines for marking
modifications in FluffyChat files.
In
`@lib/pangea/activity_sessions/activity_session_start/select_role_session_controller.dart`:
- Around line 44-104: Wrap the Pangea modifications in the
SelectRoleSessionController class (in the file
lib/pangea/activity_sessions/activity_session_start/select_role_session_controller.dart)
with the required boundary markers. Add `// `#Pangea`` comment before the
didUpdateWidget method and `// Pangea#` comment after the showStarsCard method
to denote the Pangea-modified section. Alternatively, if these changes span
multiple separate modification blocks, wrap each individual modified method or
property (such as didUpdateWidget, the getters for activityRoom, canConfirmRole,
descriptionText, selectedRoleGoals, selectedRoleCompletedGoalIds, and the
methods completedGoalIdsForRole and showStarsCard) with their own `// `#Pangea``
and `// Pangea#` markers according to your team's convention for marking
FluffyChat modifications.
- Around line 125-142: The `_confirmed` state is being set to true at the start
of the method before validation checks for `activity` and `selectedRoleId`. This
leaves the UI in a confirmed state if either validation fails and the method
returns early. Move the `setState(() => _confirmed = true);` call to after both
the `activity` null check and `selectedRoleId` null check in the
confirmRoleSelection method, so the UI only enters confirmed mode after
validation succeeds. Additionally, ensure `_confirmed` is reset to false if any
subsequent async operations fail during the role selection flow.
---
Nitpick comments:
In `@lib/pangea/activity_sessions/activity_participant_indicator.dart`:
- Around line 133-150: The star icons in the goal completion display lack
semantic labels for screen readers, making them inaccessible. Wrap each Icon
widget (the one returning either Icons.star or Icons.star_border) with a
Semantics widget that includes a label property describing the completion
status. The label should clearly indicate whether each goal is completed or not,
for example using text like "Goal completed" for the filled star icon and "Goal
not completed" for the empty star icon. This will ensure screen readers can
communicate the goal completion status to users with visual impairments.
In
`@lib/pangea/activity_sessions/activity_session_start/activity_session_state_controller.dart`:
- Around line 35-53: The scanCompletedGoalIds method performs a full room scan
on every invocation, which becomes a performance bottleneck when called
repeatedly from UI getters during rebuilds. Add memoization to this method by
caching results based on the (activityId, roleId) tuple for the current sync
snapshot. Store the cached results in a field (such as a Map or similar
structure) and invalidate the cache when the activity data or rooms list
changes. This will prevent redundant room scans within the same sync cycle while
maintaining correctness by clearing stale results when the underlying data
updates.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4985559f-cb39-4033-97fa-4b49bd5def83
📒 Files selected for processing (12)
lib/l10n/intl_en.arblib/pangea/activity_sessions/activity_participant_indicator.dartlib/pangea/activity_sessions/activity_session_chat/activity_participant_list.dartlib/pangea/activity_sessions/activity_session_start/activity_session_bottom_content.dartlib/pangea/activity_sessions/activity_session_start/activity_session_button_widget.dartlib/pangea/activity_sessions/activity_session_start/activity_session_start_page.dartlib/pangea/activity_sessions/activity_session_start/activity_session_state_controller.dartlib/pangea/activity_sessions/activity_session_start/activity_sessions_start_view.dartlib/pangea/activity_sessions/activity_session_start/confirmed_role_session_controller.dartlib/pangea/activity_sessions/activity_session_start/full_session_controller.dartlib/pangea/activity_sessions/activity_session_start/not_started_session_controller.dartlib/pangea/activity_sessions/activity_session_start/select_role_session_controller.dart
Comment on lines
+286
to
+298
| if (old.goals != widget.goals) { | ||
| final hasGoals = widget.goals != null && widget.goals!.isNotEmpty; | ||
| if (hasGoals) { | ||
| setState(() { | ||
| _displayGoals = widget.goals; | ||
| _visible = true; | ||
| _innerExpanded = true; | ||
| }); | ||
| } else { | ||
| setState(() => _visible = false); | ||
| Future.delayed(FluffyThemes.animationDuration, () { | ||
| if (mounted) setState(() => _displayGoals = null); | ||
| }); |
There was a problem hiding this comment.
Prevent delayed state clobbering in _ActivityGoalsDropdownState.didUpdateWidget.
At Line 296, the delayed callback can clear _displayGoals after newer goals arrive, causing dropdown flicker/disappearance. Also, Line 292 always expands and ignores startCollapsed during updates.
Suggested fix
class _ActivityGoalsDropdownState extends State<_ActivityGoalsDropdown> {
bool _visible = false;
bool _innerExpanded = true;
List<ActivityRoleGoal>? _displayGoals;
+ int _hideToken = 0;
@@
void didUpdateWidget(covariant _ActivityGoalsDropdown old) {
super.didUpdateWidget(old);
if (old.goals != widget.goals) {
final hasGoals = widget.goals != null && widget.goals!.isNotEmpty;
if (hasGoals) {
+ _hideToken++;
setState(() {
_displayGoals = widget.goals;
_visible = true;
- _innerExpanded = true;
+ _innerExpanded = !widget.startCollapsed;
});
} else {
+ final token = ++_hideToken;
setState(() => _visible = false);
Future.delayed(FluffyThemes.animationDuration, () {
- if (mounted) setState(() => _displayGoals = null);
+ if (!mounted || token != _hideToken) return;
+ if (widget.goals == null || widget.goals!.isEmpty) {
+ setState(() => _displayGoals = null);
+ }
});
}
}
}🤖 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/pangea/activity_sessions/activity_session_start/activity_sessions_start_view.dart`
around lines 286 - 298, The didUpdateWidget method in
_ActivityGoalsDropdownState has two issues: the delayed callback that sets
_displayGoals to null can clear newly arrived goals causing flicker, and the
code unconditionally sets _innerExpanded to true without respecting the
startCollapsed parameter. Fix the first issue by either cancelling the delayed
future if new goals arrive or by tracking the timing to prevent older delayed
operations from overwriting newer state. Fix the second issue by changing the
unconditional _innerExpanded = true assignment to respect the startCollapsed
parameter, such as by setting _innerExpanded = !widget.startCollapsed.
- cache completed goal ids
Collaborator
|
@ggurdin I've finished up changes to the activity start page! I've gone through most of the automated review, but please let me know if anything else needs to be fixed :) |
ggurdin
commented
Jun 17, 2026
| final NotStartedSessionController controller; | ||
| const _NotStartedSessionBottomContent(this.controller); | ||
|
|
||
| List<ActivitySummaryStatus> get _visibleStatuses { |
Collaborator
Author
There was a problem hiding this comment.
Nitpick, but this should be a getter function in the NotStartedSubPage enum
ggurdin
commented
Jun 17, 2026
|
|
||
| Set<String> get selectedRoleCompletedGoalIds; | ||
|
|
||
| static Set<String> scanCompletedGoalIds({ |
Collaborator
Author
There was a problem hiding this comment.
Another nitpick, but since this is a function that scans over the list of all rooms, it should go in a client extension. I don't think there's one for the orchestrator yet. I would just make it in the same directory as the orchestrator room extension.
ggurdin
commented
Jun 17, 2026
| @override | ||
| void didChangeDependencies() { | ||
| super.didChangeDependencies(); | ||
| _roomStateSubscription ??= Matrix.of(context).client.onRoomState.stream |
Collaborator
Author
There was a problem hiding this comment.
I don't think listening to roomState like this is a good idea. This stream includes all roomState updates for all rooms the user is in, which means if could be firing off constantly for a user in many rooms. If should be filtered down to only the target room, and only the target event types, i.e.:
_roomStateSubscription ??= Matrix.of(context).client.onRoomState.stream
.where(
(u) =>
u.roomId == widget.roomId &&
u.state.type == PangeaEventTypes.orchestratorAwardedGoals,
)
.listen((_) {
if (mounted) setState(() => _completedGoalIdsCache = {});
});
ggurdin
commented
Jun 17, 2026
Comment on lines
+51
to
+52
| Map<String, Set<String>> _completedGoalIdsCache = {}; | ||
| StreamSubscription? _roomStateSubscription; |
Collaborator
Author
There was a problem hiding this comment.
There's some repeated code related to these variables in this file, confirmed_role_session_controller.dart, and select_role_session_controller.dart. Is it possible to move most of that into ActivitySessionStateController? Each of the controllers will still have to call functions to init and dispose the subscription, since ActivitySessionStateController is not a stateful widget, but everything else can be handled inside of ActivitySessionStateController
Collaborator
Author
|
@avashilling Hey, looks good for the most part! I left a couple of comments |
- some other small cleanup tweaks
Collaborator
|
@ggurdin updated! |
Collaborator
Author
|
@avashilling Thanks! A couple small things: You don't need to pass the rooms list to And in the |
- and remove unnecessary room list
10 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Thank you so much for your contribution to FluffyChat ❤️❤️❤️
Pull Request has been tested on:
Summary by CodeRabbit
Release Notes
New Features
Navigation / Actions
Bug Fixes / UI
Localization