Skip to content
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.

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

[webview_flutter_wkwebview] Add a listener for the canGoBack property change on the iOS platform. #8203

Merged
merged 16 commits into from
Jan 9, 2025

Conversation

StanleyCocos
Copy link
Contributor

@StanleyCocos StanleyCocos commented Dec 2, 2024

Added a listener callback for the canGoBack property on iOS. Since it can only be effectively implemented on iOS, it is only exposed in the WKWebView layer.

Fixes flutter/flutter#158160

Pre-launch Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I read the [Tree Hygiene] page, which explains my responsibilities.
  • I read and followed the [relevant style guides] and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/packages repo does use dart format.)
  • I signed the [CLA].
  • The title of the PR starts with the name of the package surrounded by square brackets, e.g. [shared_preferences]
  • I [linked to at least one issue that this PR fixes] in the description above.
  • I updated pubspec.yaml with an appropriate new version according to the [pub versioning philosophy], or this PR is [exempt from version changes].
  • I updated CHANGELOG.md to add a description of the change, [following repository CHANGELOG style], or this PR is [exempt from CHANGELOG changes].
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is [test-exempt].
  • All existing and new tests are passing.

@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!).

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. The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group.

@cbracken
Copy link
Member

Hi @StanleyCocos thanks for your patch. I looks like this change is causing existing tests to fail and does not include any new test code. Once you've fixed both those issues, please let me know and I'll take a look!

@StanleyCocos
Copy link
Contributor Author

@cbracken
Thank you for your suggestion.

I’ve added a test case and fixed the integration test error. However, there seems to be another issue caused by not updating the version. I'm not sure what I should do to fix this. Could you give me some advice?

@StanleyCocos
Copy link
Contributor Author

can you review again

cc @cbracken

@cbracken
Copy link
Member

One last fix to make to get CI passing:

Changed packages: webview_flutter_wkwebview
[0:00] Running for packages/webview_flutter/webview_flutter_wkwebview...
  No version change.
No version change found, but the change to this package could not be verified to be exempt
from version changes according to repository policy.
If this is a false positive, please comment in the PR to explain why the PR
is exempt, and add (or ask your reviewer to add) the "override: no versioning needed" label.


The following packages had errors:
  packages/webview_flutter/webview_flutter_wkwebview:
    Missing version change
See above for full details.

@StanleyCocos
Copy link
Contributor Author

I seem to understand that the version information needs to be adjusted. But I don't know how to make the changes. Or should I apply for the "no version control required" tag?

@StanleyCocos

This comment was marked as resolved.

@StanleyCocos
Copy link
Contributor Author

can you review again
cc @cbracken

Copy link
Member

@cbracken cbracken left a comment

Choose a reason for hiding this comment

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

This lgtm. Adding @stuartmorgan as second reviewer.

@cbracken cbracken requested a review from stuartmorgan January 7, 2025 18:09
@stuartmorgan stuartmorgan requested review from bparrishMines and removed request for stuartmorgan January 7, 2025 19:13
@stuartmorgan
Copy link
Contributor

@bparrishMines would be a better second reviewer here.

@bparrishMines bparrishMines changed the title [webview_flutter] Add a listener for the canGoBack property change on the iOS platform. [webview_flutter_wkwebview] Add a listener for the canGoBack property change on the iOS platform. Jan 9, 2025
Copy link
Contributor

@bparrishMines bparrishMines left a comment

Choose a reason for hiding this comment

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

LGTM

I slightly updated the changelog message and changed it to a minor version bump since this is a new feature.

@bparrishMines bparrishMines added the autosubmit Merge PR when tree becomes green via auto submit App label Jan 9, 2025
@auto-submit auto-submit bot merged commit 6554751 into flutter:main Jan 9, 2025
77 checks passed
berhili098 added a commit to berhili098/packages that referenced this pull request Jan 9, 2025
… change on the iOS platform. (flutter#8203)

Added a listener callback for the canGoBack property on iOS. Since it can only be effectively implemented on iOS, it is only exposed in the WKWebView layer.

Fixes flutter/flutter#158160

*Replace this paragraph with a description of what this PR is changing or adding, and why. Consider including before/after screenshots.*

*List which issues are fixed by this PR. You must list at least one issue.*

## Pre-launch Checklist

- [ ] I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
- [ ] I read the [Tree Hygiene] page, which explains my responsibilities.
- [ ] I read and followed the [relevant style guides] and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/packages repo does use `dart format`.)
- [ ] I signed the [CLA].
- [ ] The title of the PR starts with the name of the package surrounded by square brackets, e.g. `[shared_preferences]`
- [ ] I [linked to at least one issue that this PR fixes] in the description above.
- [ ] I updated `pubspec.yaml` with an appropriate new version according to the [pub versioning philosophy], or this PR is [exempt from version changes].
- [ ] I updated `CHANGELOG.md` to add a description of the change, [following repository CHANGELOG style], or this PR is [exempt from CHANGELOG changes].
- [ ] I updated/added relevant documentation (doc comments with `///`).
- [ ] I added new tests to check the change I am making, or this PR is [test-exempt].
- [ ] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on [Discord].

<!-- Links -->
[Contributor Guide]: https://github.com/flutter/packages/blob/main/CONTRIBUTING.md
[Tree Hygiene]: https://github.com/flutter/flutter/blob/master/docs/contributing/Tree-hygiene.md
[relevant style guides]: https://github.com/flutter/packages/blob/main/CONTRIBUTING.md#style
[CLA]: https://cla.developers.google.com/
[Discord]: https://github.com/flutter/flutter/blob/master/docs/contributing/Chat.md
[linked to at least one issue that this PR fixes]: https://github.com/flutter/flutter/blob/master/docs/contributing/Tree-hygiene.md#overview
[pub versioning philosophy]: https://dart.dev/tools/pub/versioning
[exempt from version changes]: https://github.com/flutter/flutter/blob/master/docs/ecosystem/contributing/README.md#version
[following repository CHANGELOG style]: https://github.com/flutter/flutter/blob/master/docs/ecosystem/contributing/README.md#changelog-style
[exempt from CHANGELOG changes]: https://github.com/flutter/flutter/blob/master/docs/ecosystem/contributing/README.md#changelog
[test-exempt]: https://github.com/flutter/flutter/blob/master/docs/contributing/Tree-hygiene.md#tests
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 9, 2025
github-merge-queue bot pushed a commit to flutter/flutter that referenced this pull request Jan 9, 2025
flutter/packages@3fc6b7a...6554751

2025-01-09 [email protected] [webview_flutter_wkwebview] Add a
listener for the canGoBack property change on the iOS platform.
(flutter/packages#8203)
2025-01-09 [email protected] [flutter_svg]
Implement errorBuilder callback (flutter/packages#8364)
2025-01-08 [email protected]
[interactive_media_ads] Adds remaining methods for internal wrapper of
the iOS native `IMAAdDisplayContainer`. (flutter/packages#8153)
2025-01-08 [email protected] [go_router_builder] Add a fallback for
a not null List or Set param (flutter/packages#8349)
2025-01-08 [email protected] Manual roll Flutter (stable)
from 8495dee to 17025dd (2 revisions) (flutter/packages#8396)
2025-01-08 [email protected]
[ci][video_player_android][camera_android_camerax] Manual roll and fix
tests (flutter/packages#8391)
2025-01-07 [email protected] [ci] Run web tests in wasm (unit +
integration). (flutter/packages#8111)
2025-01-07 [email protected] [camera_platform_interface] Add API support query
for image streaming (flutter/packages#8307)
2025-01-07 [email protected]
[camera_android_camerax][video_player_android] Suppresses deprecation of
the `SurfaceProducer.Callback.onSurfaceDestroyed` method
(flutter/packages#8388)
2025-01-07 [email protected] Bump Remaining Plugin Example Apps to
targetSdkVersion >= 34 and compileSdkVersion >= 34
(flutter/packages#8361)
2025-01-07 [email protected] Replace
`onSurfaceCreated` with `onSurfaceAvailable`. (flutter/packages#8384)

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-flutter-autoroll
Please CC [email protected] on the revert to ensure that a
human
is aware of the problem.

To file a bug in Flutter:
https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autosubmit Merge PR when tree becomes green via auto submit App p: webview_flutter platform-ios
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[webview_flutter] Added callback for canGoBack property change
4 participants