Re-invoke DismissIntent in Autocomplete if ignored #99403
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!). If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
flutter-dashboard
bot
added
the
framework
markusaksli-nc
changed the title
|
Couldn't figure out a test right now, will take another look tomorrow |
justinmc
left a comment
justinmc
left a comment
There was a problem hiding this comment.
I think it should be possible to test by wrapping Autocomplete in Actions and hitting escape when the options are not visible. Let me know if not when you get back to this though. Otherwise this PR looks like exactly what I was expecting, thanks for opening it so quick.
|
@justinmc that did the trick, made sure it kills the mutant as well. |
justinmc
left a comment
justinmc
left a comment
There was a problem hiding this comment.
LGTM 👍
One nit to make the test more comprehensive, assuming it works like I'm thinking it does. Up to you if you want to add it or not, though.
There was a problem hiding this comment.
Nit: What if you added one more step here for completeness: Long press the text and assert that the text selection toolbar shows up. Press escape and assert that it goes away, the options are still visible, and wrappingActionInvoked is false. Then proceed with the rest of the test.
That should be how it works, right?
There was a problem hiding this comment.
Doesn't seem within the scope of this test for me. This fix doesn't really affect the text selection toolbar dismiss behavior in an Autocomplete so the added asserts wouldn't fail either way.
markusaksli-nc
force-pushed
the
autocomplete-intent-fix
branch
from
86e72b5 to
d32eb3c
Compare
3 participants
Ensures the
DismissIntentused to dismiss theRawAutocompletemenu is re-invoked if the menu isn't visible.Pre-launch Checklist
///).