Skip to content

[CP-stable]Refactor route focus node creation #149378

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.

Sign up for GitHub

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

Jump to bottom
Merged
auto-submit merged 1 commit into from
May 31, 2024
Merged

[CP-stable]Refactor route focus node creation #149378

auto-submit merged 1 commit into from
May 31, 2024

Conversation

@flutteractionsbot
Copy link

@flutteractionsbot flutteractionsbot commented May 31, 2024 •
edited by gspencergoog
Loading

Issue Link:

#148867

Cherry-picking #147390

Changelog Description:

Fixes a focus issue that causes TextFields to not function after cupertino back swipes

Impact Description:

iOS, macos, iOS web, macos web app that uses TextField

Workaround:

disable cupertino back swipe

Risk:

What is the risk level of this cherry-pick?

  • Low
  • Medium
  • High

Test Coverage:

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

  • Yes
  • No

Validation Steps:

What are the steps to validate that this fix works?

TextField should not be broken after a cupertino back swipe

@gspencergoog
Refactor route focus node creation (flutter#147390)
a40d444 
## Description

This fixes an issue in the creation of the `FocusScope` in a route:  the route should be creating the `FocusScope` widget it has with `withExternalFocusNode`, since it is modifying the node attributes in a builder. 

Also modified some `AnimatedBuilder`s to be `ListenableBuilder`s, since they're not using animations (no functionality change there, since the implementation of the two is identical).

## Related Issues
 - flutter#147256
 - Fixes flutter#146844

## Tests
 - Updated example test.
@flutteractionsbot flutteractionsbot added the cp: review Cherry-picks in the review queue label May 31, 2024
@flutteractionsbot
Copy link
Author

flutteractionsbot commented May 31, 2024

@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. d: api docs Issues with https://api.flutter.dev/ d: examples Sample code and demos f: routes Navigator, Router, and related APIs. labels May 31, 2024
@jmagman
Copy link
Member

jmagman commented May 31, 2024

@chunhtai or @gspencergoog can you fill out this cherry-pick PR description? There didn't seem to be any follow-up to #149178 (comment).

For future reference, the cherry-pick automated process with labels is at:
https://github.com/flutter/flutter/blob/master/docs/releases/Flutter-Cherrypick-Process.md#automatically-creates-a-cherry-pick-request

@jmagman jmagman requested review from chunhtai and gspencergoog May 31, 2024 02:02
chunhtai
chunhtai reviewed May 31, 2024
View reviewed changes
Copy link
Contributor

@chunhtai chunhtai left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM if also LGT @gspencergoog

gspencergoog
gspencergoog approved these changes May 31, 2024
View reviewed changes
Copy link
Contributor

@gspencergoog gspencergoog left a comment

Choose a reason for hiding this comment

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

32384589-a60f0e74-c078-11e7-9bc1-e5b5287aea9d

@gspencergoog gspencergoog added the autosubmit Merge PR when tree becomes green via auto submit App label May 31, 2024
@auto-submit auto-submit bot merged commit d8e72c6 into flutter:flutter-3.22-candidate.0 May 31, 2024
@jmagman
Copy link
Member

jmagman commented May 31, 2024

@itsjustkevin heads up this was merged instead of going through the release team process.

@techouse
Copy link
Contributor

techouse commented Jun 1, 2024 •
edited
Loading

@jmagman @itsjustkevin should #149178 now get closed?

@darshankawar darshankawar mentioned this pull request Jun 3, 2024
Closed
@huycozy huycozy mentioned this pull request Jun 4, 2024
Closed
@itsjustkevin itsjustkevin mentioned this pull request Jun 4, 2024
Closed
@jmagman
Copy link
Member

jmagman commented Jun 4, 2024

@jmagman @itsjustkevin should #149178 now get closed?

https://github.com/flutter/flutter/blob/master/docs/releases/Flutter-Cherrypick-Process.md#lifecycle-of-a-cherry-pick

Once the cherry-pick has been added to a release, the release engineering team will close the cherry-pick issue.

@BazinC
Copy link

BazinC commented Jun 5, 2024 •
edited
Loading

Hi flutter team, what's required next? how long does it usually take after the merge of CP-stable to release a stable hotfix version of Flutter? (3.22.2?)
Thank you 🙂
(This regression is so painful... Our team upgraded from 3.19 -> 3.22, then had to downgrade because of this in order to keep releasing our app, and having to keep developing for both version is not productive at all even with the help of FVM)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 6, 2024
@engine-flutter-autoroll
d8e72c6d1 [CP-stable]Refactor route focus node creation (flutter/flut…
50b96d6 
…ter#149378)
@christopherfujino
Copy link
Contributor

christopherfujino commented Jun 6, 2024

Hi flutter team, what's required next? how long does it usually take after the merge of CP-stable to release a stable hotfix version of Flutter? (3.22.2?) Thank you 🙂 (This regression is so painful... Our team upgraded from 3.19 -> 3.22, then had to downgrade because of this in order to keep releasing our app, and having to keep developing for both version is not productive at all even with the help of FVM)

Stable 3.22.2 is now out

quyenlv-unicloud
quyenlv-unicloud reviewed Jul 19, 2024
View reviewed changes
packages/flutter/lib/src/widgets/routes.dart
AnimatedBuilder(
animation: widget.route.navigator?.userGestureInProgressNotifier ?? ValueNotifier<bool>(false),
ListenableBuilder(
listenable: widget.route.navigator?.userGestureInProgressNotifier ?? ValueNotifier<bool>(false),
Copy link

@quyenlv-unicloud quyenlv-unicloud Jul 19, 2024

Choose a reason for hiding this comment

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

Hi, do we need to dispose ValueNotifier<bool>(false),?

@itsjustkevin
Copy link
Contributor

itsjustkevin commented Jul 19, 2024

Hi @quyenlv-unicloud, if you are running into problems, please create a new issue referencing this PR over commenting.

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

Reviewers

@gspencergoog gspencergoog gspencergoog approved these changes

@chunhtai chunhtai chunhtai approved these changes

+1 more reviewer

@quyenlv-unicloud quyenlv-unicloud quyenlv-unicloud left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

autosubmit Merge PR when tree becomes green via auto submit App cp: review Cherry-picks in the review queue d: api docs Issues with https://api.flutter.dev/ d: examples Sample code and demos f: routes Navigator, Router, and related APIs. framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

@flutteractionsbot @jmagman @techouse @BazinC @christopherfujino @itsjustkevin @gspencergoog @chunhtai @quyenlv-unicloud