Skip to content

Conversation

@bleroux
Copy link
Contributor

@bleroux bleroux commented Apr 19, 2023

Description

This PR introduces DefaultSelectionStyle.mouseCursor to configure the mouse cursor over selectable text.
It also applies this solution to InkResponse to make the mouse cursor win over the default one provided by selectable Text for many Material components (such as buttons).

Before

Before.mov

After

After.mov

Related Issue

Fixes #104595

Tests

Adds 6 tests.

@flutter-dashboard flutter-dashboard bot added a: text input Entering text in a text field or keyboard related problems f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. labels Apr 19, 2023
@bleroux bleroux force-pushed the make_selectable_test_mouse_cursor_configurable branch from dce745e to 80711d5 Compare April 19, 2023 13:56
@bleroux bleroux force-pushed the make_selectable_test_mouse_cursor_configurable branch from 80711d5 to 0978640 Compare April 19, 2023 21:10
@guidezpl
Copy link
Member

Nice work! However there are quite a few components with clickable parts that include text. These include:

  • FAB / extended FAB
  • Chips
  • Tabs
  • Navigation (rail, drawer, bar)
  • Top app bar
  • XListTiles (e.g. CheckboxListTile)
  • Menus
  • Date and time pickers

Rather than handle these individually, can we add it a level below, say to InkWell and see which components still need to be handled individually?

@bleroux
Copy link
Contributor Author

bleroux commented Apr 20, 2023

Rather than handle these individually, can we add it a level below, say to InkWell and see which components still need to be handled individually?

I agree with this. Let me try to update this PR.
First goal of this PR was to get feedback for the DefaultSelectionStyle.mouseCursor solution I proposed. I choosed Material buttons to illustrate how this can be used but it makes sense to use it for InkWell to reduce code duplication.

@bleroux bleroux force-pushed the make_selectable_test_mouse_cursor_configurable branch 2 times, most recently from 8302a39 to e66a9b3 Compare April 20, 2023 12:14
@bleroux bleroux requested review from HansMuller and guidezpl April 20, 2023 13:12
Copy link
Member

@guidezpl guidezpl left a comment

Choose a reason for hiding this comment

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

Nice, please update PR description.

  _    ___ _____ __  __ 
 | |  / __|_   _|  \/  |
 | |_| (_ | | | | |\/| |
 |____\___| |_| |_|  |_|
                        

Comment on lines 85 to 86
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// The [MouseCursor] for mouse pointers that are hovering over selectable
/// Text widgets.
/// The [MouseCursor] for mouse pointers hovering over selectable [Text] widgets.

Comment on lines +47 to +48
Copy link
Member

Choose a reason for hiding this comment

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

@bleroux bleroux force-pushed the make_selectable_test_mouse_cursor_configurable branch from e66a9b3 to 2a86178 Compare April 21, 2023 11:04
@bleroux bleroux added the autosubmit Merge PR when tree becomes green via auto submit App label Apr 21, 2023
@auto-submit auto-submit bot merged commit d4f884e into flutter:master Apr 21, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 21, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 21, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 22, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 22, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 23, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 23, 2023
@bleroux bleroux deleted the make_selectable_test_mouse_cursor_configurable branch April 24, 2023 12:10
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 24, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 24, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 24, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 24, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 25, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 25, 2023
@reza00farjam
Copy link

Thanks for fix. On which channel and which version this fix is available?

@bleroux
Copy link
Contributor Author

bleroux commented Apr 27, 2023

This fix is available on the master channel.

@guidezpl
Copy link
Member

guidezpl commented Apr 27, 2023

pro tip: d4f884e indicates the branches the merge commit is present on

image

@LucaIaconelli
Copy link

Thanks for the fix. Is it also available in stable or only on master?

@guidezpl
Copy link
Member

As long as stable isn't listed on that commit, it's not available. Since beta releases are monthly, expect it in the next one.

@LucaIaconelli
Copy link

@guidezpl still not available on stable?

@federicopolesello
Copy link

@LucaIaconelli I see that is present in stable from version 3.13.0

https://docs.flutter.dev/release/release-notes/release-notes-3.13.0

@reza00farjam
Copy link

For future reference (both for myself and others), here's an example of how this output can be utilized effectively:

SelectionArea(
  child: DefaultSelectionStyle.merge(
    mouseCursor: SystemMouseCursors.click,
    child: Text('Clickable and selectable text'),
  ),
);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a: text input Entering text in a text field or keyboard related problems autosubmit Merge PR when tree becomes green via auto submit App f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Should SelectionArea cursor lose to click cursor?

5 participants