Repository navigation
[CP-stable]Add ContextMenuButtonKind - #195015
Conversation
part of flutter#193527 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [AI contribution guidelines] and understand my responsibilities, or I am not using AI tools. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant in-code documentation (doc comments with `///`). - [x] If this PR introduces a new feature or capability, I created and linked a website documentation issue or PR in [flutter/website] (or verified none is needed). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing.
|
@LouiseHsu please fill out the PR description above, afterwards the release team will review this request. |
|
This pull request was opened from and to a release candidate branch. This should only be done as part of the official Flutter release process. If you are attempting to make a regular contribution to the Flutter project, please close this PR and follow the instructions at Tree Hygiene for detailed instructions on contributing to Flutter. Reviewers: Use caution before merging pull requests to release branches. Ensure the proper procedure has been followed. |
There was a problem hiding this comment.
Code Review
This pull request introduces ContextMenuButtonKind to replace ContextMenuButtonType to allow adding new button kinds without breaking exhaustive switches. It updates ContextMenuButtonItem, EditableText, SelectableRegion, and SystemContextMenu to use the new class while maintaining legacy support. Feedback on the changes suggests improving the copyWith method in ContextMenuButtonItem to preserve the original _type and _kind representations when they are not being overridden.
justinmc
left a comment
There was a problem hiding this comment.
Confirming this is pretty low risk.
@LouiseHsu what's your end goal with the translate button and 3.5x? Are you sure you won't need additional framework changes?
As long as this is the only needed cherry pick for flutter/flutter then this looks good to me.
justinmc
left a comment
There was a problem hiding this comment.
@LouiseHsu confirmed offline that this is the only cherry pick needed, and follow up will happen on master, so this LGTM 👍
|
@LouiseHsu can I get you to update the changlog part of the pr to the style we use? |
reidbaker
left a comment
There was a problem hiding this comment.
need the pr description to contain a changelog entry that follows our style guide
|
@LouiseHsu here's a suggested changelog entry for the PR description, written to our hotfix best practices. Feel free to use it as-is or edit it: |
done |
3157269
into
flutter:flutter-3.47-candidate.0
Adds CHANGELOG.md entries for the 3.47.9 stable hotfix (#195073, #194959, #195015). CI-only cherry-picks are omitted. Tracker: #195427 ### Edits to cherry-pick changelog descriptions - #195073 and #195015: used verbatim from the cherry-pick PR's "Changelog Description". - #194959 (@bkonyi): **edited** to follow [Hotfix Documentation Best Practices](https://github.com/flutter/flutter/blob/main/docs/releases/Hotfix-Documentation-Best-Practices.md) (describe the state prior to the fix): - Original: "When running `flutter test` in pub workspaces, monorepos, or symlinked directories, fix a StateError crash during native assets compilation by falling back to appName matching." - Changelog: "When running `flutter test` in pub workspaces, monorepos, or symlinked directories on any platform, native assets compilation crashed with a `StateError`." - Changes: rewrote as the pre-fix behavior, added "on any platform", dropped the implementation detail ("falling back to appName matching"). @bkonyi FYI — please let us know if the edited wording for #194959 looks right.
This pull request is created by [automatic cherry pick workflow](https://github.com/flutter/flutter/blob/main/docs/releases/Flutter-Cherrypick-Process.md#automatically-creates-a-cherry-pick-request) Cherry-pick of #193528 (already cherry-picked to `3.47.9` stable in #195015, ensuring `3.50` does not regress public API shipped in `3.47.9`). Also includes `e58c176fe4` / `7d0aca2908` (`+12/-1`, test-only) restoring the `no-shuffle` tag on `packages/flutter/test/material/text_field_test.dart` (reverting #193550 and restoring `no-shuffle` in `packages/flutter/dart_test.yaml`) because `3.50` branched before #194256 fixed #194008 (`Theme._kFallbackTheme` platform caching across shuffled `TargetPlatformVariant` tests). ### Issue Link: #193527 #194008 ### Impact Description: `ContextMenuButtonKind` and `ContextMenuButtonItem.kind` shipped in Flutter `3.47.9` stable (`#195015`) after `flutter-3.50-candidate.0` branched. Without cherry-picking `#193528` into `flutter-3.50-candidate.0`, packages and apps adopting `ContextMenuButtonKind` on `3.47.9` stable (including `cupertino_ui` and `material_ui` for iOS Translate context menu support in `#180021`) would fail to compile on `3.50.0`. The change itself is purely additive and does not deprecate `ContextMenuButtonType` or alter existing behavior. Restoring `no-shuffle` on `text_field_test.dart` unblocks `framework_tests_libraries` and `Mac framework_tests_impeller` on `flutter-3.50-candidate.0` under daily shuffled test seeds. ### Changelog Description: - [flutter/193527](#193527) When building custom text selection menus, packages can't add new actions such as Translate without breaking exhaustive switches on `ContextMenuButtonType`; adds the extensible `ContextMenuButtonKind`. ### Workaround: No. ### Risk: What is the risk level of this cherry-pick? ### Test Coverage: Are you confident that your fix is well-tested by automated tests? ### Validation Steps: Run `flutter test test/widgets/context_menu_button_item_test.dart test/widgets/editable_text_test.dart test/widgets/selectable_region_test.dart test/widgets/system_context_menu_test.dart` and `flutter test --test-randomize-ordering-seed=20261103 --fatal-warnings test/material/text_field_test.dart` to verify `ContextMenuButtonKind` backwards compatibility and `text_field_test.dart` execution under shuffled seeds.
This pull request is created by automatic cherry pick workflow
Please fill in the form below, and a flutter domain expert will evaluate this cherry pick request.
Issue Link:
Part of #193527
Impact Description:
Low user-facing impact on its own, as this PR is purely additive (
ContextMenuButtonKindandContextMenuButtonItem.kind) without deprecatingContextMenuButtonTypeor changing existing behavior. However, cherry-picking this into3.47unblocks the decoupledcupertino_uiandmaterial_uipackages (which must support the latest stable Flutter release) from migrating toContextMenuButtonKindand unblocks shipping the iOS Translate context menu work (#180021) without waiting an extra stable release cycle.Changelog Description:
Explain this cherry pick:
ContextMenuButtonType; adds the extensibleContextMenuButtonKind.Workaround:
Is there a workaround for this issue?
No.
Risk:
What is the risk level of this cherry-pick?
Test Coverage:
Are you confident that your fix is well-tested by automated tests?
Validation Steps:
Run the automated unit tests in
flutter test test/widgets/context_menu_button_item_test.dart,test/widgets/editable_text_test.dart,test/widgets/selectable_region_test.dart, andtest/widgets/system_context_menu_test.dartto verifyContextMenuButtonKindandContextMenuButtonItem.kindbehavior and backwards compatibility withContextMenuButtonType.