Fix: The enableFeedback property of InkWell cannot be set to a nullab… #158907
Conversation
github-actions
bot
added
framework
This comment was marked as resolved.
This comment was marked as resolved.
|
@StanleyCocos |
There was a problem hiding this comment.
Looks great overall, but @TahaTesser is right—this PR will need either a test or a test exemption. (One could argue that there are virtually no semantic changes here, but in my experience, requesting a test exemption for a borderline case hasn't succeeded.)
One approach could be to keep the bool? enableFeedback = true for now and add an assert; then test whether the assert fires (perhaps this isn't the best idea, since we'd have to ignore tighten_type_of_initializing_formals), and then a while after it lands in stable we could get back in here and finish the job.
|
Yes, you're right. However, as a developer, I believe that if someone explicitly calls InkWell(enableFeedback: null), it is an error. For developers, setting it to null likely implies an intention to set it to false, but it will actually default to true. Therefore, we think it would be more intuitive to notify them that there's an error here. Using assert(enableFeedback != null) is a runtime reminder, but developers may not immediately notice this issue. For developers who set enableFeedback != null, this change would not affect them. |
|
Fortunately, this PR isn't a breaking change per our Tree Hygiene rules, since all checks are passing. In my personal opinion, I'd be happy to see this merged right away. However, the bot comment above regarding tests needs to be resolved first. This seems to me like it could qualify as a "code refactor with no semantic changes"—feel free to head over to Discord (specifically the |
|
@nate-thegrate If you could help me resolve the robot comments above, it would be greatly appreciated. Thank you! |
|
Sounds good! I should have an update for you within the next few days :) |
|
I'm very happy to see this outcome. Thank you |
|
test-exempt: code refactor with no semantic change |
Interestingly, at the time this file was migrated to null-safety and the /// The [mouseCursor], [enableFeedback], and [excludeFromSemantics] arguments
/// must not be null. |
|
Interesting, I guess this must have slipped under the radar! @StanleyCocos we'll be able to merge this pull request once it has 2 approvals. |
bleroux
left a comment
bleroux
left a comment
There was a problem hiding this comment.
LGTM! Thanks for the cleanup. 🙏
nate-thegrate
added
the
autosubmit
flutter-dashboard
bot
removed
the
autosubmit
Roll Flutter from 8536b96 to 93d772c (37 revisions) flutter/flutter@8536b96...93d772c 2024-11-21 [email protected] Added additional logging to `_listCoreDevices` (flutter/flutter#159275) 2024-11-21 [email protected] Roll Flutter Engine from 78b87f3fe023 to d1a08064e193 (1 revision) (flutter/flutter#159280) 2024-11-21 [email protected] Shut down DevTools and DDS processes if flutter_tools is killed by a signal (flutter/flutter#159238) 2024-11-21 [email protected] Remove `RepaintBoundary` that is no longer needed. (flutter/flutter#159232) 2024-11-21 [email protected] Try a speculative fix for Gradle OOMs. (flutter/flutter#159234) 2024-11-21 [email protected] On-device Widget Inspector button exits widget selection (flutter/flutter#158219) 2024-11-21 [email protected] Roll Flutter Engine from 523d381893c8 to 78b87f3fe023 (2 revisions) (flutter/flutter#159270) 2024-11-21 [email protected] Remove `firebase_abstract_method_smoke_test` (flutter/flutter#159145) 2024-11-21 [email protected] Roll Packages from e95f6d8 to 913b99e (7 revisions) (flutter/flutter#159268) 2024-11-21 [email protected] Roll Flutter Engine from 69c325513a65 to 523d381893c8 (3 revisions) (flutter/flutter#159263) 2024-11-21 [email protected] [flutter_tools] opt iOS/macOS apps out of Metal API validation via migrator, update templates in repo. (flutter/flutter#159228) 2024-11-21 [email protected] Scribe Android handwriting text input (flutter/flutter#148784) 2024-11-21 [email protected] Terminate non-detached test devices on `flutter run` completion (flutter/flutter#159170) 2024-11-21 [email protected] Un-skip tests that use `flutter build apk`. (flutter/flutter#159231) 2024-11-20 [email protected] Roll Flutter Engine from 2d32cf3a7971 to 69c325513a65 (1 revision) (flutter/flutter#159229) 2024-11-20 [email protected] Roll Flutter Engine from 3828681d1f86 to 2d32cf3a7971 (3 revisions) (flutter/flutter#159226) 2024-11-20 [email protected] Roll Flutter Engine from 3245c8976976 to 3828681d1f86 (1 revision) (flutter/flutter#159217) 2024-11-20 [email protected] Add `--dry-run` to `dev/bots/test.dart`. (flutter/flutter#158956) 2024-11-20 [email protected] Add docs for setting up Android Studio to auto format Kotlin code (flutter/flutter#159209) 2024-11-20 [email protected] Roll Flutter Engine from 80d77505fdde to 3245c8976976 (1 revision) (flutter/flutter#159214) 2024-11-20 [email protected] Fix: The enableFeedback property of InkWell cannot be set to a nullab� (flutter/flutter#158907) 2024-11-20 [email protected] Roll Flutter Engine from 3f19207e820e to 80d77505fdde (1 revision) (flutter/flutter#159210) 2024-11-20 [email protected] Roll Packages from fc4adc7 to e95f6d8 (6 revisions) (flutter/flutter#159201) 2024-11-20 [email protected] Remove dependency on [Target] and instead operate on [Architecture] (flutter/flutter#159196) 2024-11-20 [email protected] Fix git command in Quality-Assurance.md (flutter/flutter#155146) 2024-11-20 [email protected] Roll Flutter Engine from 7eb87547cbc6 to 3f19207e820e (4 revisions) (flutter/flutter#159176) 2024-11-20 [email protected] Make `runner` non-nullable as it always is. (flutter/flutter#159156) 2024-11-19 [email protected] Update Material 3 `CircularProgressIndicator` for new visual style (flutter/flutter#158104) 2024-11-19 [email protected] Roll Flutter Engine from 4ff696b555dc to 7eb87547cbc6 (3 revisions) (flutter/flutter#159168) 2024-11-19 [email protected] Add platform-android label for all flutter_tools *android* files (flutter/flutter#159166) 2024-11-19 [email protected] Roll Flutter Engine from d5820a638885 to 4ff696b555dc (1 revision) (flutter/flutter#159164) 2024-11-19 [email protected] Reland Add UI Benchmarks (flutter/flutter#153368) 2024-11-19 [email protected] fix lint usage of `task` inside `resolve_dependecies.gradle` file (flutter/flutter#158022) 2024-11-19 [email protected] Roll Flutter Engine from cff1e751f853 to d5820a638885 (5 revisions) (flutter/flutter#159155) 2024-11-19 [email protected] Removing redundant backticks in `flutter\packages\flutter_tools\gradle\gradle.kts` (flutter/flutter#159051) 2024-11-19 [email protected] Fixes initial validation with AutovalidateMode.always on first build (flutter/flutter#156708) 2024-11-19 [email protected] Introduce new Material 3 `Slider` shapes (flutter/flutter#152237) 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],[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 ...
5 participants
I found that enableFeedback can be set to null, but in actual use, setting it to null has no effect and is overwritten by true. I think this is an issue.
Call Error