-
Notifications
You must be signed in to change notification settings - Fork 28.6k
Add mouseCursor
parameter to ReorderableListView
#160246
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.
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
Add mouseCursor
parameter to ReorderableListView
#160246
Conversation
@dkwingsmt is this what you were expecting for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @ValentinVignal, glad to see you!
Flyby comment from triage, it looks like the docs test shard is failing.
Thank you for the notice @Piinks. I'm not sure this PR is valid or what @dkwingsmt expected. Maybe we'll close it. I was thinking of putting more efforts into this once it is confirmed we are moving forward with it :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, this PR somehow missed my attention. The WidgetState mouse cursor looks good to me, but I think the approach can be much simpler.
/// If [mouseCursor] is a [WidgetStateMouseCursor], | ||
/// [WidgetStateProperty.resolve] is used for the following [WidgetState]s: | ||
/// | ||
/// * [WidgetState.hovered]. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hovered should not be needed since cursors don't make sense unless hovered :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense, I've simplified it in refactor: Simplify code
WidgetState.hovered: SystemMouseCursors.grab, | ||
}), | ||
<WidgetState>{ | ||
if (_dragIndex.value == index) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we really need this? We only need to know whether the widget is being dragged at all, because the mouse will definitely be on the item being dragged.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right! I made the code simpler in refactor: Simplify code
229a19b
to
318d573
Compare
Hey @ValentinVignal, noticing in triage here, it looks like this PR is failing some checks. Can you take a look? |
8edd40b
to
362d510
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you for the contribution!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM to me as well! Thank you!
362d510
to
7f9d04d
Compare
Rebased to resolve a CI issue. |
Roll Flutter from 4b23b81 to 864d4f5 (50 revisions) flutter/flutter@4b23b81...864d4f5 2025-01-10 [email protected] Mark complex_layout_scroll_perf_macos__timeline_summary unflaky (flutter/flutter#160997) 2025-01-10 [email protected] Mark hello_world_macos__compile unflaky (flutter/flutter#160998) 2025-01-10 [email protected] Mark animated_complex_opacity_perf_macos__e2e_summary unflaky (flutter/flutter#160996) 2025-01-10 [email protected] Mark integration_ui_test_test_macos unflaky (flutter/flutter#160999) 2025-01-10 [email protected] Mark hot_mode_dev_cycle_macos_target__benchmark unflaky (flutter/flutter#161000) 2025-01-10 [email protected] Add a virtual-display (VD) platform view test, and refactor tests a bit. (flutter/flutter#161349) 2025-01-10 [email protected] Remove `CIRRUS_TASK_NAME` from what I can tell, is always omitted on `LUCI` (flutter/flutter#161391) 2025-01-10 [email protected] Replace the always omitted `CPU` environment variable with `numberOfProcessors`. (flutter/flutter#161392) 2025-01-10 [email protected] We no longer have a separate engine repo. (flutter/flutter#161400) 2025-01-10 [email protected] Update Style-guide-for-Flutter-repo.md (flutter/flutter#161344) 2025-01-09 [email protected] integration_test: Add gitignore of golden image (flutter/flutter#161404) 2025-01-09 [email protected] Fix link to engine docs in CONTRIBUTING.md (flutter/flutter#161401) 2025-01-09 [email protected] Roll Packages from 3fc6b7a to 6554751 (11 revisions) (flutter/flutter#161379) 2025-01-09 [email protected] Add `mouseCursor` parameter to `ReorderableListView` (flutter/flutter#160246) 2025-01-09 [email protected] Remove Cirrus CI from Flutter goldens. (flutter/flutter#161396) 2025-01-09 [email protected] remove`useMaterial3: true,` in from the template (flutter/flutter#160525) 2025-01-09 [email protected] Remove `accept_android_sdk_licenses.sh`, which appears unused. (flutter/flutter#161388) 2025-01-09 [email protected] Roll Dart to Version 3.7.0-312.0.dev (flutter/flutter#161394) 2025-01-09 [email protected] Marks Linux analyzer_benchmark to be flaky (flutter/flutter#161307) 2025-01-09 [email protected] [Impeller] fix scaling of trampoline import of GLES textures into Vulkan. (flutter/flutter#161331) 2025-01-09 [email protected] Support DDC library bundle format and remove support for DDC module format (flutter/flutter#161276) 2025-01-09 [email protected] Remove seemingly stale web Cirrus and "Web Installer" instructions (flutter/flutter#161389) 2025-01-09 [email protected] Proposal to deprecate `webGoldenComparator`. (flutter/flutter#161196) 2025-01-09 [email protected] `ImplicitlyAnimatedWidgetState` code cleanup (flutter/flutter#160567) 2025-01-09 [email protected] Exclude `*texture*` as matching for `a: text input` (flutter/flutter#161354) 2025-01-09 [email protected] [Impeller] add opt in flag for SurfaceControl testing. (flutter/flutter#161353) 2025-01-09 [email protected] Roll Dart to Version 3.7.0-307.0.dev (flutter/flutter#161278) 2025-01-09 [email protected] remove formatter from snippet tool (flutter/flutter#161347) 2025-01-09 [email protected] Refactor keyboard manager tests (flutter/flutter#160637) 2025-01-09 [email protected] [flutter_tools] ignore viewpost ime and samsung spam messages. (flutter/flutter#161199) 2025-01-09 [email protected] Revert "[SwiftPM] Add separate feature flag for the app migration (#158897)" (flutter/flutter#161342) 2025-01-09 [email protected] git ignore .ccls-cache (flutter/flutter#161340) 2025-01-08 98614782+auto-submit[bot]@users.noreply.github.com Reverts "[SwiftPM] Turn on by default (#161275)" (flutter/flutter#161339) 2025-01-08 [email protected] Update engine instructions for monorepo (flutter/flutter#161184) 2025-01-08 [email protected] [Impeller] re-enable Adreno 630 (flutter/flutter#161287) 2025-01-08 [email protected] [SwiftPM] Turn on by default (flutter/flutter#161275) 2025-01-08 [email protected] Make the encoding of a `YamlNode` to a `String` more explicit. (flutter/flutter#161270) 2025-01-08 [email protected] Normalize the translation column of the color matrix. (flutter/flutter#161109) 2025-01-08 [email protected] Rename `native_driver` to `android_{driver_extensions|engine_test}` (flutter/flutter#161263) 2025-01-08 [email protected] [Impeller] reland: fix porterduff shader and handle optimized out texture binding in GLES backend. (flutter/flutter#161326) 2025-01-08 [email protected] Updating AVD Dependency for Android 28 Emulator (flutter/flutter#160978) 2025-01-08 [email protected] Adds onHover and onLongPress to IconButton widget (flutter/flutter#160032) 2025-01-08 [email protected] Marks Linux linux_desktop_impeller to be unflaky (flutter/flutter#161302) 2025-01-08 98614782+auto-submit[bot]@users.noreply.github.com Reverts "[Impeller] porter duff workarounds for Adreno GPU. (#161273)" (flutter/flutter#161318) 2025-01-08 [email protected] Revert "fixed keyboardDismissBehavior on scroll without a drag" (flutter/flutter#161277) 2025-01-08 [email protected] Revert "use uuid from package:uuid instead of from package:usage" (flutter/flutter#161292) ...
Reason for revert: Causing the tree to go red. See #161815. |
Time to revert pull request flutter/flutter/160246 has elapsed. |
The test failure that this PR caused was fixed in: #161836 |
Funny coincidence actually... This PR was perfect 👍 The test failure had existed for a long time, but it never came to light until a random daily reordering triggered it, but only in the tests added in this PR. |
Part of #58192 (comment)
Screen.Recording.2024-12-13.at.11.37.02.PM.mov
Screen.Recording.2024-12-13.at.11.41.04.PM.mov
Pre-launch Checklist
///
).If you need help, consider asking for advice on the #hackers-new channel on Discord.