Skip to content

Fix Action Mode stop errors and small-screen layout - #9056

Merged
jigar-f merged 3 commits into
jigar/action-mode-refactorfrom
atavism/action-mode-refactor
Sep 15, 2026
Merged

jigar-f merged 3 commits into
jigar/action-mode-refactorfrom
atavism/action-mode-refactor

Conversation

@atavism

@atavism atavism commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Show an error when Action Mode fails to stop, while keeping the sharing state accurate and allowing another attempt.

  • Restore scrolling and a minimum globe height so large text doesn’t cause overflow or overlapping content.
  • Let waiting and arrival messages wrap on narrow screens.

@atavism
atavism requested review from jigar-f and a lite review from Copilot September 15, 2026 11:05
@coderabbitai

coderabbitai Bot commented Sep 15, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 83558abb-8121-4045-a42c-43692b205a3a

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Comment @coderabbitai help to get the list of available commands.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Stop-error messaging and stale error-state handling still need correction.

Pull request overview

Fixes Action Mode stop-error handling and improves small-screen layout behavior.

Changes:

  • Displays stop failures while preserving retryability.
  • Restores scrolling, minimum globe height, and message wrapping.
  • Adds widget coverage and regenerates provider hashes.
File summaries
File Reviewed changes
test/features/action_mode/action_mode_widgets_test.dart Stop-error and small-screen regression tests
lib/features/home/provider/data_cap_info_provider.g.dart Regenerated provider hash
lib/features/home/provider/app_event_notifier.g.dart Regenerated provider hash
lib/features/action_mode/provider/share_notifier.g.dart Regenerated provider hash
lib/features/action_mode/peer_status_pill.dart Enables narrow-screen text wrapping
lib/features/action_mode/action_mode.dart Updates error handling and responsive layout
Review details

Files not reviewed (3)

  • lib/features/action_mode/provider/share_notifier.g.dart: Generated file
  • lib/features/home/provider/app_event_notifier.g.dart: Generated file
  • lib/features/home/provider/data_cap_info_provider.g.dart: Generated file

Suppressed comments (2)

lib/features/action_mode/action_mode.dart:92

  • This reuses the "Couldn't share" translation for a failed stop, even though the state remains active and the toggle is intentionally still on for retry. That makes the status contradictory and suggests starting/sharing failed; use a stop-specific localized message (including the backend detail) for this path.
    final error = state.errorMessage != null
        ? 'smc_status_error_with_message'.i18n.fill([state.errorMessage!])
        : 'smc_status_error_generic'.i18n;

lib/features/action_mode/action_mode.dart:89

  • A failed stop leaves errorMessage set, and _applyUnboundedSnapshot later updates active/mode without clearing that message when the backend reports enabled: false. This predicate will therefore keep rendering the old stop error even though the switch is off; only treat the message as an error while the session is still active (or clear it when the snapshot disables sharing).
    final hasError =
        state.phase == SharePhase.error ||
        (state.errorMessage?.isNotEmpty ?? false);
  • Files reviewed: 3/6 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

// The globe fills whatever the cards leave over; the page only scrolls
// when the cards alone are taller than the screen.
return SafeArea(
child: CustomScrollView(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@atavism One reason I choose Slivers is their memory efficiency: for unbounded lists, we must redraw every second, but Slivers offer more flexibility and use less memory.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jigar-f Great point! I opened a PR here to restore the use of Silvers: #9063

@jigar-f jigar-f left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@atavism Just one comment, I think that's worth looking into.

@jigar-f
jigar-f merged commit 0d11f3b into jigar/action-mode-refactor Sep 15, 2026
3 checks passed
@jigar-f
jigar-f deleted the atavism/action-mode-refactor branch September 15, 2026 14:09
jigar-f added a commit that referenced this pull request Sep 15, 2026
* Refactored action mode and fixed UI issues.

* Fix redraw issue.

* Date action card UI

* code review updates

* Restore Action Mode status comment (#9056)

---------

Co-authored-by: atavism <paul@getlantern.org>
Co-authored-by: atavism <atavism@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants