Skip to content

Conversation

@chunhtai
Copy link
Contributor

waiting for flutter/flutter#139164

Presubmit checklist

@flutter-website-bot
Copy link
Collaborator

flutter-website-bot commented Nov 29, 2023

@atsansone atsansone added review.copy Awaiting Copy Review act.await-dev-pr Needs dev PR to merge before merging docs labels Nov 30, 2023
@sfshaza2 sfshaza2 removed the review.copy Awaiting Copy Review label Dec 1, 2023
Copy link
Contributor

@sfshaza2 sfshaza2 left a comment

Choose a reason for hiding this comment

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

Let me know if any of my changes affect the meaning you intended, @chunhtai!


## Timeline

Landed in version: TBD
Copy link
Contributor

Choose a reason for hiding this comment

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

Please fill this out.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will update after the change is merged

@sfshaza2
Copy link
Contributor

sfshaza2 commented Dec 4, 2023

Also, @chunhtai, please add this breaking change to the index page under "Not yet released to stable".

thx!

@chunhtai
Copy link
Contributor Author

will reopen once i am back from vacation

@chunhtai chunhtai closed this Jan 16, 2024
@chunhtai chunhtai reopened this Apr 12, 2024

## Timeline

Landed in version: TBD
Copy link
Contributor Author

Choose a reason for hiding this comment

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

will fill this out when pr merges. It would require internal migration, so may take some time

@chunhtai chunhtai requested a review from sfshaza2 April 12, 2024 17:44
Copy link
Contributor

@sfshaza2 sfshaza2 left a comment

Choose a reason for hiding this comment

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

Overall, looks very good! Just a few fixes to smooth out the language.

Co-authored-by: Shams Zakhour (ignore Sfshaza) <[email protected]>
@chunhtai chunhtai requested a review from sfshaza2 April 12, 2024 22:19
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Apr 18, 2024
Adds a generic type and pop result to popscope and its friend.

The use cases are to be able to capture the result when the pop is called.

migration guide: flutter/website#9872
auto-submit bot added a commit to flutter/flutter that referenced this pull request Apr 18, 2024
Reverts: #139164
Initiated by: chunhtai
Reason for reverting: hard breaking change
Original PR Author: chunhtai

Reviewed By: {justinmc}

This change reverts the following previous change:
Adds a generic type and pop result to popscope and its friend.

The use cases are to be able to capture the result when the pop is called.

migration guide: flutter/website#9872
gilnobrega pushed a commit to gilnobrega/flutter that referenced this pull request Apr 22, 2024
Adds a generic type and pop result to popscope and its friend.

The use cases are to be able to capture the result when the pop is called.

migration guide: flutter/website#9872
gilnobrega pushed a commit to gilnobrega/flutter that referenced this pull request Apr 22, 2024
…lutter#147015)

Reverts: flutter#139164
Initiated by: chunhtai
Reason for reverting: hard breaking change
Original PR Author: chunhtai

Reviewed By: {justinmc}

This change reverts the following previous change:
Adds a generic type and pop result to popscope and its friend.

The use cases are to be able to capture the result when the pop is called.

migration guide: flutter/website#9872
@sfshaza2 sfshaza2 changed the title [DO NOT MERGE] Adds popscope migration guide Adds popscope migration guide Apr 26, 2024
Copy link
Contributor

@sfshaza2 sfshaza2 left a comment

Choose a reason for hiding this comment

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

lgtm


## Summary

Added a generic type in [`PopScope`][] class and updated the [`onPopInvoked`][]
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Added a generic type in [`PopScope`][] class and updated the [`onPopInvoked`][]
Added a generic type in the [`PopScope`][] class and updated the [`onPopInvoked`][]

Comment on lines 16 to 17
was called. The generic type is added to the `PopScope` class
so that `onPopInvoked` can access the type-safe result.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
was called. The generic type is added to the `PopScope` class
so that `onPopInvoked` can access the type-safe result.
was called. The generic type added to the `PopScope` class
enables `onPopInvoked` access the type-safe result.

@sfshaza2 sfshaza2 merged commit 1ad57ee into flutter:main Apr 26, 2024
atsansone pushed a commit to atsansone/website that referenced this pull request Apr 29, 2024
waiting for flutter/flutter#139164

## Presubmit checklist

- [ ] This PR doesn’t contain automatically generated corrections
(Grammarly or similar).
- [ ] This PR follows the [Google Developer Documentation Style
Guidelines](https://developers.google.com/style) — for example, it
doesn’t use _i.e._ or _e.g._, and it avoids _I_ and _we_ (first person).
- [ ] This PR uses [semantic line
breaks](https://github.com/dart-lang/site-shared/blob/main/doc/writing-for-dart-and-flutter-websites.md#semantic-line-breaks)
of 80 characters or fewer.

---------

Co-authored-by: Shams Zakhour (ignore Sfshaza) <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

act.await-dev-pr Needs dev PR to merge before merging docs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants