-
Notifications
You must be signed in to change notification settings - Fork 29.7k
FloatingActionButton: add themeable mouse cursor #103473
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
Conversation
|
Regardless of numerous upstream merges, #96714 has been repeatedly failing on unrelated customer tests for a couple of months. This is an attempt to run through the CI with the same changes on a fresh and clean base. |
|
@werainkhatri This PR replaces #96714 which refuses to pass customer this. |
werainkhatri
left a comment
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.
weird how that happens... maybe it should be informed to the infra team?
anyway, this LGTM with a few nits.
packages/flutter/test/material/floating_action_button_theme_test.dart
Outdated
Show resolved
Hide resolved
packages/flutter/lib/src/material/floating_action_button_theme.dart
Outdated
Show resolved
Hide resolved
|
@jpnurmi Will you be able to continue working on this? It's totally fine if not; we can take it over (though it might take longer). I'm just asking because I'm trying to clean out our review queue. |
jpnurmi
left a comment
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.
I thought I had left answers but I guess I never pressed the button. Anyway, I have merged the latest main.
packages/flutter/lib/src/material/floating_action_button_theme.dart
Outdated
Show resolved
Hide resolved
packages/flutter/test/material/floating_action_button_theme_test.dart
Outdated
Show resolved
Hide resolved
|
@jpnurmi looks like there are a few failing tests still, can you take another look? thanks! |
Co-authored-by: Viren Khatri <[email protected]>
8592480 to
48b6193
Compare
|
Rebase helped to get the latest customer tests run. |
werainkhatri
left a comment
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 ✅
Allow themes to override FloatingActionButton's mouse cursor.
This is #96714 reopened as a new PR and is a partial fix to #88371.
Pre-launch Checklist
///).