Skip to content

[iOS] Do not hide selection handles when platform hides system context menu #168678

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
auto-submit merged 3 commits into from
May 13, 2025
Merged

[iOS] Do not hide selection handles when platform hides system context menu #168678

auto-submit merged 3 commits into from
May 13, 2025

Conversation

@Renzo-Olivares
Copy link
Contributor

@Renzo-Olivares Renzo-Olivares commented May 12, 2025 •
edited
Loading

Fixes an issue where selection handles would be hidden when the system hides the platform context menu on iOS.

Before:

Screen.Recording.2025-05-12.at.11.55.56.AM.mov

After:

Screen.Recording.2025-05-12.at.11.55.06.AM.mov

Fixes #168636

Pre-launch Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I read the [Tree Hygiene] wiki page, which explains my responsibilities.
  • I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement].
  • I signed the [CLA].
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is [test-exempt].
  • I followed the [breaking change policy] and added [Data Driven Fixes] where supported.
  • All existing and new tests are passing.

@github-actions github-actions bot added framework flutter/packages/flutter repository. See also f: labels. a: text input Entering text in a text field or keyboard related problems f: material design flutter/packages/flutter/material repository. f: cupertino flutter/packages/flutter/cupertino repository labels May 12, 2025
@Renzo-Olivares Renzo-Olivares force-pushed the fix-ios-sys-edit-menu branch from 3176909 to 87a579c Compare May 12, 2025 19:00
@Renzo-Olivares Renzo-Olivares marked this pull request as ready for review May 12, 2025 19:00
@Renzo-Olivares Renzo-Olivares requested a review from justinmc May 12, 2025 19:00
@Renzo-Olivares
Copy link
Contributor Author

Renzo-Olivares commented May 12, 2025

There is another interesting issue I noticed with the system context menu while working on this PR.

On the framework side we have implemented toggling the selection toolbar when tapping on the current selection. This does not work with the system context menu as seen in the video in this PR's description, tapping the selection hides the toolbar but immediately shows it again after. This happens because first the system dismisses the platform context menu when the tap happens outside the context menu (tap on the selection), the framework then responds to this by hiding the context menu. The framework also then responds to the tap on the selection and tries to toggle the toolbar, this ends up bringing up the toolbar again. The expected behavior would have been for the tap on the selection to hide the toolbar and not reshow it until a subsequent tap. I'll open an issue for this, this PR only solves the selection handles being hidden.

loic-sharma
loic-sharma reviewed May 13, 2025
View reviewed changes
packages/flutter/lib/src/widgets/system_context_menu.dart
),
items: items ?? getDefaultItems(editableTextState),
onSystemHide: editableTextState.hideToolbar,
onSystemHide: () => editableTextState.hideToolbar(false),
Copy link
Member

@loic-sharma loic-sharma May 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Hm in hindsight it'd be nice if this was a named argument instead of a positional argument. It's hard to tell that boolean sets the handle visibility 👀

Copy link
Contributor Author

@Renzo-Olivares Renzo-Olivares May 13, 2025

Choose a reason for hiding this comment

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

Definitely agree with this. I kind of wish the hideToolbar method didn't do this at all and that there was a separate method to hide the handles, or that the method had a more appropriate name. SelectionOverlay and TextSelectionOverlay expose a hideHandles but we don't expose it at the editable text level.

loic-sharma
loic-sharma approved these changes May 13, 2025
View reviewed changes
Copy link
Member

@loic-sharma loic-sharma left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this so quick! Also, great tests :)

@Renzo-Olivares Renzo-Olivares added the autosubmit Merge PR when tree becomes green via auto submit App label May 13, 2025
@auto-submit auto-submit bot added this pull request to the merge queue May 13, 2025
@Renzo-Olivares
Copy link
Contributor Author

Renzo-Olivares commented May 13, 2025

Thanks for the review!

Merged via the queue into flutter:master with commit cba2e86 May 13, 2025
76 checks passed
@flutter-dashboard flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label May 13, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 14, 2025
@engine-flutter-autoroll
cba2e8635 [iOS] Do not hide selection handles when platform hides sys…
2580c73 
…tem context menu (flutter/flutter#168678)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request May 14, 2025
Merged
auto-submit bot pushed a commit to flutter/packages that referenced this pull request May 14, 2025
@engine-flutter-autoroll
Roll Flutter from 336a7ecf7b40 to 0b9f9288caf3 (22 revisions) (#9251)
9230536 
flutter/flutter@336a7ec...0b9f928

2025-05-14 [email protected] Fix Linux Impeller support broken by incorrect deletion of stencil buffer. (flutter/flutter#168668)
2025-05-14 [email protected] Roll Fuchsia Linux SDK from 6J81agNhuK4q255Jj... to fSvuEJgRmHxnewRJr... (flutter/flutter#168794)
2025-05-14 [email protected] Run `{Platform} flutter_packaging` builders on release candidates. (flutter/flutter#168762)
2025-05-14 [email protected] Remove `docs_deploy_beta`, fix `docs_publish`, add comments. (flutter/flutter#168754)
2025-05-14 [email protected] Add `titleAlignment` to `CheckboxListTile` and `RadioListTile`  (flutter/flutter#168666)
2025-05-14 [email protected] Remove deprecated todo about caching (flutter/flutter#168534)
2025-05-13 [email protected] Replace hardcoded host and app level build.gradle paths with `AndroidProject`-level getters in `gradle_errors.dart` (flutter/flutter#167949)
2025-05-13 [email protected] Improve documentation for KeyedSubtree constructor (flutter/flutter#167198)
2025-05-13 [email protected] Wire up the system text scaler from `PlatformDispatcher` (flutter/flutter#159999)
2025-05-13 [email protected] Roll Dart SDK from 645d04a7a964 to b3520981e0f0 (3 revisions) (flutter/flutter#168721)
2025-05-13 [email protected] [a11y] Semanctis flag refactor step 3: framework part (flutter/flutter#167771)
2025-05-13 [email protected] Remove/replace the `team` label with `c: contributor-productivity`. (flutter/flutter#168702)
2025-05-13 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Clip search artifacts in CupertinoSliverNavigationBar searchable-to-searchable transitions (#168510)" (flutter/flutter#168775)
2025-05-13 [email protected] Use live region in error text input decorator for Android (flutter/flutter#165531)
2025-05-13 [email protected] [tool] Fix deprecated API calls within tool (flutter/flutter#168200)
2025-05-13 [email protected] Clip search artifacts in CupertinoSliverNavigationBar searchable-to-searchable transitions (flutter/flutter#168510)
2025-05-13 [email protected] Fixes hero not shown when remove pages without animation (flutter/flutter#168617)
2025-05-13 [email protected] [iOS] Do not hide selection handles when platform hides system context menu (flutter/flutter#168678)
2025-05-13 [email protected] Updated execution path to silently include --start-paused and updated tests (flutter/flutter#168400)
2025-05-13 [email protected] Roll Packages from 6a28ad9 to 2e166de (2 revisions) (flutter/flutter#168736)
2025-05-13 [email protected] [web] Fix multiline input selection in Chrome. (flutter/flutter#168217)
2025-05-13 [email protected] Call xcode_backend.dart from macos_assemble.sh (flutter/flutter#168108)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC [email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
FMorschel pushed a commit to FMorschel/packages that referenced this pull request Jun 9, 2025
@engine-flutter-autoroll @FMorschel
Roll Flutter from 336a7ecf7b40 to 0b9f9288caf3 (22 revisions) (flutte…
5cba342 
…r#9251)

flutter/flutter@336a7ec...0b9f928

2025-05-14 [email protected] Fix Linux Impeller support broken by incorrect deletion of stencil buffer. (flutter/flutter#168668)
2025-05-14 [email protected] Roll Fuchsia Linux SDK from 6J81agNhuK4q255Jj... to fSvuEJgRmHxnewRJr... (flutter/flutter#168794)
2025-05-14 [email protected] Run `{Platform} flutter_packaging` builders on release candidates. (flutter/flutter#168762)
2025-05-14 [email protected] Remove `docs_deploy_beta`, fix `docs_publish`, add comments. (flutter/flutter#168754)
2025-05-14 [email protected] Add `titleAlignment` to `CheckboxListTile` and `RadioListTile`  (flutter/flutter#168666)
2025-05-14 [email protected] Remove deprecated todo about caching (flutter/flutter#168534)
2025-05-13 [email protected] Replace hardcoded host and app level build.gradle paths with `AndroidProject`-level getters in `gradle_errors.dart` (flutter/flutter#167949)
2025-05-13 [email protected] Improve documentation for KeyedSubtree constructor (flutter/flutter#167198)
2025-05-13 [email protected] Wire up the system text scaler from `PlatformDispatcher` (flutter/flutter#159999)
2025-05-13 [email protected] Roll Dart SDK from 645d04a7a964 to b3520981e0f0 (3 revisions) (flutter/flutter#168721)
2025-05-13 [email protected] [a11y] Semanctis flag refactor step 3: framework part (flutter/flutter#167771)
2025-05-13 [email protected] Remove/replace the `team` label with `c: contributor-productivity`. (flutter/flutter#168702)
2025-05-13 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Clip search artifacts in CupertinoSliverNavigationBar searchable-to-searchable transitions (#168510)" (flutter/flutter#168775)
2025-05-13 [email protected] Use live region in error text input decorator for Android (flutter/flutter#165531)
2025-05-13 [email protected] [tool] Fix deprecated API calls within tool (flutter/flutter#168200)
2025-05-13 [email protected] Clip search artifacts in CupertinoSliverNavigationBar searchable-to-searchable transitions (flutter/flutter#168510)
2025-05-13 [email protected] Fixes hero not shown when remove pages without animation (flutter/flutter#168617)
2025-05-13 [email protected] [iOS] Do not hide selection handles when platform hides system context menu (flutter/flutter#168678)
2025-05-13 [email protected] Updated execution path to silently include --start-paused and updated tests (flutter/flutter#168400)
2025-05-13 [email protected] Roll Packages from 6a28ad9 to 2e166de (2 revisions) (flutter/flutter#168736)
2025-05-13 [email protected] [web] Fix multiline input selection in Chrome. (flutter/flutter#168217)
2025-05-13 [email protected] Call xcode_backend.dart from macos_assemble.sh (flutter/flutter#168108)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC [email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
@justinmc justinmc mentioned this pull request Jun 20, 2025
Merged
Ortes pushed a commit to Ortes/packages that referenced this pull request Jun 25, 2025
@engine-flutter-autoroll @Ortes
Roll Flutter from 336a7ecf7b40 to 0b9f9288caf3 (22 revisions) (flutte…
8acf26e 
…r#9251)

flutter/flutter@336a7ec...0b9f928

2025-05-14 [email protected] Fix Linux Impeller support broken by incorrect deletion of stencil buffer. (flutter/flutter#168668)
2025-05-14 [email protected] Roll Fuchsia Linux SDK from 6J81agNhuK4q255Jj... to fSvuEJgRmHxnewRJr... (flutter/flutter#168794)
2025-05-14 [email protected] Run `{Platform} flutter_packaging` builders on release candidates. (flutter/flutter#168762)
2025-05-14 [email protected] Remove `docs_deploy_beta`, fix `docs_publish`, add comments. (flutter/flutter#168754)
2025-05-14 [email protected] Add `titleAlignment` to `CheckboxListTile` and `RadioListTile`  (flutter/flutter#168666)
2025-05-14 [email protected] Remove deprecated todo about caching (flutter/flutter#168534)
2025-05-13 [email protected] Replace hardcoded host and app level build.gradle paths with `AndroidProject`-level getters in `gradle_errors.dart` (flutter/flutter#167949)
2025-05-13 [email protected] Improve documentation for KeyedSubtree constructor (flutter/flutter#167198)
2025-05-13 [email protected] Wire up the system text scaler from `PlatformDispatcher` (flutter/flutter#159999)
2025-05-13 [email protected] Roll Dart SDK from 645d04a7a964 to b3520981e0f0 (3 revisions) (flutter/flutter#168721)
2025-05-13 [email protected] [a11y] Semanctis flag refactor step 3: framework part (flutter/flutter#167771)
2025-05-13 [email protected] Remove/replace the `team` label with `c: contributor-productivity`. (flutter/flutter#168702)
2025-05-13 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Clip search artifacts in CupertinoSliverNavigationBar searchable-to-searchable transitions (#168510)" (flutter/flutter#168775)
2025-05-13 [email protected] Use live region in error text input decorator for Android (flutter/flutter#165531)
2025-05-13 [email protected] [tool] Fix deprecated API calls within tool (flutter/flutter#168200)
2025-05-13 [email protected] Clip search artifacts in CupertinoSliverNavigationBar searchable-to-searchable transitions (flutter/flutter#168510)
2025-05-13 [email protected] Fixes hero not shown when remove pages without animation (flutter/flutter#168617)
2025-05-13 [email protected] [iOS] Do not hide selection handles when platform hides system context menu (flutter/flutter#168678)
2025-05-13 [email protected] Updated execution path to silently include --start-paused and updated tests (flutter/flutter#168400)
2025-05-13 [email protected] Roll Packages from 6a28ad9 to 2e166de (2 revisions) (flutter/flutter#168736)
2025-05-13 [email protected] [web] Fix multiline input selection in Chrome. (flutter/flutter#168217)
2025-05-13 [email protected] Call xcode_backend.dart from macos_assemble.sh (flutter/flutter#168108)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC [email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 14, 2025
@engine-flutter-autoroll
cba2e8635 [iOS] Do not hide selection handles when platform hides sys…
528407e 
…tem context menu (flutter/flutter#168678)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 14, 2025
@engine-flutter-autoroll
cba2e8635 [iOS] Do not hide selection handles when platform hides sys…
01bb93f 
…tem context menu (flutter/flutter#168678)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 15, 2025
@engine-flutter-autoroll
cba2e8635 [iOS] Do not hide selection handles when platform hides sys…
ac11bdd 
…tem context menu (flutter/flutter#168678)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 15, 2025
@engine-flutter-autoroll
cba2e8635 [iOS] Do not hide selection handles when platform hides sys…
1bf3738 
…tem context menu (flutter/flutter#168678)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 16, 2025
@engine-flutter-autoroll
cba2e8635 [iOS] Do not hide selection handles when platform hides sys…
1527d5b 
…tem context menu (flutter/flutter#168678)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@loic-sharma loic-sharma loic-sharma approved these changes

@justinmc justinmc Awaiting requested review from justinmc

Assignees

No one assigned

Labels

a: text input Entering text in a text field or keyboard related problems f: cupertino flutter/packages/flutter/cupertino repository f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[iOS] Selection handles sometimes disappear while quickly dragging in TextField/CupertinoTextField

2 participants

@Renzo-Olivares @loic-sharma