Skip to content

[CP-stable]Update FocusManager platform check to include iOS - #148976

Closed
flutteractionsbot wants to merge 1 commit into
flutter:flutter-3.22-candidate.0from
flutteractionsbot:cp-stable-8d955cdd644edd089d5545e83615d68b01d62c1c
Closed

flutteractionsbot wants to merge 1 commit into
flutter:flutter-3.22-candidate.0from
flutteractionsbot:cp-stable-8d955cdd644edd089d5545e83615d68b01d62c1c

Conversation

@flutteractionsbot

@flutteractionsbot flutteractionsbot commented May 23, 2024 •

Copy link
Copy Markdown
Contributor

Issue Link:

#148475

Changelog Description:

On iOS, selecting an email/password autofill suggestion should populate the input fields.

Impact Description:

In iOS apps, selecting an autofill suggestion does not populate the input fields.

Workaround:

No

Risk:

What is the risk level of this cherry-pick?

  • Low
  • Medium
  • High

This essentially reverses #142930 on non-desktop platforms.

Test Coverage:

Are you confident that your fix is well-tested by automated tests?

  • Yes
  • No

Though this flow could really use platform-specific integration tests (cc @nate-thegrate)

Validation Steps:

Code sample and instructions at #148475 (comment)

Both iOS and Android run into issues when the FocusManager starts responding to app lifecycle changes.

Fortunately, this feature is primarily meant for desktop platforms, so the problem can be resolved with a platform check.

fixes flutter#148475
@flutteractionsbot flutteractionsbot added the cp: review Cherry-picks in the review queue label May 23, 2024
@flutteractionsbot

Copy link
Copy Markdown
Contributor Author

@jmagman please fill out the PR description above, afterwards the release team will review this request.

@github-actions github-actions Bot added framework flutter/packages/flutter repository. See also f: labels. f: focus Focus traversal, gaining or losing focus labels May 23, 2024
ChangeNotifier.maybeDispatchObjectCreation(this);
}
if (kIsWeb || defaultTargetPlatform != TargetPlatform.android) {
final bool shouldListenToAppLifecycle = switch (defaultTargetPlatform) {

@LongCatIsLooong LongCatIsLooong May 23, 2024 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@nate-thegrate Sorry but it looks like I missed it in the original review: why's this ignoring kIsWeb now?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks like the test is failing too.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good catch, thank you!

@nate-thegrate nate-thegrate left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@jmagman Would it be okay to close this PR in favor of #148984?

@jmagman

jmagman commented May 23, 2024

Copy link
Copy Markdown
Member

Given the revert, closing. Based on my "What is the risk level of this cherry-pick? --> Low" answer being totally off, I think this might be better to let this bake in master a bit instead of hotfixing.

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

Labels

cp: review Cherry-picks in the review queue f: focus Focus traversal, gaining or losing focus framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants