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] Added callback for canGoBack property change #158160

Open
StanleyCocos opened this issue Nov 5, 2024 · 18 comments · May be fixed by flutter/packages#8203
Open

[webview_flutter] Added callback for canGoBack property change #158160

StanleyCocos opened this issue Nov 5, 2024 · 18 comments · May be fixed by flutter/packages#8203
Labels
c: new feature Nothing broken; request for a new capability c: proposal A detailed proposal for a change to Flutter p: webview The WebView plugin P3 Issues that are less important to the Flutter project package flutter/packages repository. See also p: labels. team-ecosystem Owned by Ecosystem team triaged-ecosystem Triaged by Ecosystem team

Comments

@StanleyCocos
Copy link
Contributor

Use case

I found that using WebKitWebViewController.setAllowsBackForwardNavigationGestures(true); does not work properly.
I hope to add a callback for changes in the canGoBack property to distinguish whether WebKitWebViewController.setAllowsBackForwardNavigationGestures(false); needs to be prohibited

Proposal

Added callback for canGoBack property change
I'd be happy to create a PR if I can.

@StanleyCocos StanleyCocos changed the title [packages, webview_flutter]: Added callback for canGoBack property change [webview_flutter] Added callback for canGoBack property change Nov 5, 2024
@huycozy huycozy added the in triage Presently being triaged by the triage team label Nov 5, 2024
@huycozy
Copy link
Member

huycozy commented Nov 5, 2024

Hi @StanleyCocos
The use case doesn't seem convincing or maybe I'm not understanding this proposal. Can you elaborate?

If you see a bug with setAllowsBackForwardNavigationGestures, please file a bug report.

Also, I see canGoBack has been supported here already: https://github.com/flutter/packages/blob/main/packages/webview_flutter/webview_flutter/lib/src/webview_controller.dart#L191

@huycozy huycozy added the waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds label Nov 5, 2024
@StanleyCocos
Copy link
Contributor Author

As far as I know, the swiping back of webview conflicts with the return of MaterialPageRoute. The solution is to conditionally determine PopScop, as shown in the following example

Currently only passive acquisition is supported. I think it should support change notifications like setOnUrlChange.

@override
  Widget build(BuildContext context) {
    return PopScope(
      canPop: !_canPop,
      onPopInvokedWithResult: _canPop ? (_, __) => Future.value(true) : null,
      child: Scaffold(
        appBar: AppBar(
          backgroundColor: Theme.of(context).colorScheme.inversePrimary,
          title: Text('canGoBack: $_canPop'),
          actions: [
            IconButton(
              onPressed: () async {
                _canPop = await _controller.canGoBack();
                print("canPop: $_canPop");
                setState(() {});
              },
              icon: const Icon(Icons.update),
            ),
          ],
        ),
        body: WebViewWidget(controller: _controller),
      ),
    );
  }
_SimulatorScreenRecordingiPhone15ProMax2024110.mp4

@github-actions github-actions bot removed the waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds label Nov 6, 2024
@huycozy
Copy link
Member

huycozy commented Nov 6, 2024

The above issue doesn't sound similar to what you requested originally. To implement setOnUrlChange, you already can do it inside NavigationDelegate, see the sample below:

Demo video (iPhone 7, iOS 15.8)
RPReplay_Final1730866676.mp4
Sample code
import 'package:flutter/material.dart';
import 'package:webview_flutter/webview_flutter.dart';
import 'package:webview_flutter_android/webview_flutter_android.dart';
import 'package:webview_flutter_wkwebview/webview_flutter_wkwebview.dart';

void main() => runApp(const MaterialApp(home: WebViewExample()));

class WebViewExample extends StatefulWidget {
  const WebViewExample({super.key});

  @override
  State<WebViewExample> createState() => _WebViewExampleState();
}

class _WebViewExampleState extends State<WebViewExample> {
  late final WebViewController controller;
  String? _url;

  @override
  void initState() {
    super.initState();
    controller = WebViewController()
      ..setJavaScriptMode(JavaScriptMode.unrestricted)
      ..setBackgroundColor(const Color(0x00000000))
      ..setNavigationDelegate(
        NavigationDelegate(
          onProgress: (int progress) {
            debugPrint('WebView is loading (progress : $progress%)');
          },
          onPageStarted: (String url) {},
          onPageFinished: (String url) {},
          onWebResourceError: (WebResourceError error) {},
          onUrlChange: (UrlChange change) {
            debugPrint('URL changed: ${change.url}');
            setState(() {
              _url = change.url;
            });
          },
          onNavigationRequest: (request) => NavigationDecision.navigate,
        ),
      )
      ..loadRequest(Uri.parse('https://www.flutter.dev/'));

    // #docregion platform_features
    if (controller.platform is AndroidWebViewController) {
      AndroidWebViewController.enableDebugging(true);
      (controller.platform as AndroidWebViewController)
          .setMediaPlaybackRequiresUserGesture(false);
    } else if (controller.platform is WebKitWebViewController) {
      (controller.platform as WebKitWebViewController)
          .setAllowsBackForwardNavigationGestures(true);
    }
  }

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(title: const Text('Flutter Simple Example')),
      body: Column(
        children: [
          Padding(
            padding: const EdgeInsets.all(4.0),
            child: Text('Url changed: $_url}'),
          ),
          Expanded(child: WebViewWidget(controller: controller)),
        ],
      ),
    );
  }
}

If you found a bug with canGoBack as the implementation above, please file a bug report with a complete sample code.

Added callback for canGoBack property change

If you persist with this proposal, please elaborate on the use case.

@huycozy huycozy added the waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds label Nov 6, 2024
@StanleyCocos
Copy link
Contributor Author

StanleyCocos commented Nov 6, 2024

Thank you for your reply.
I think I wasn't clear enough in my expression.
The feature I want to add is onCanGoBackChange, similar to onUrlChange. It doesn't require us to actively check; instead, it will proactively notify me of any changes.
Because I need to know in real-time on the WebView page whether it is on the initial H5 page, since if it is on the H5 initial page, I need the swipe-back gesture to navigate to the parent WebView page, rather than the parent H5 page.

Sample code
import 'package:flutter/material.dart';
import 'package:webview_flutter/webview_flutter.dart';
import 'package:webview_flutter_android/webview_flutter_android.dart';
import 'package:webview_flutter_wkwebview/webview_flutter_wkwebview.dart';


void main() => runApp(const MyApp());

class MyApp extends StatelessWidget {
  const MyApp({super.key});
  @override
  Widget build(context) => const MaterialApp(home: Page1());
}

class Page1 extends StatefulWidget {
  const Page1({super.key});

  @override
  State<StatefulWidget> createState() => _Page1State();
}


class _Page1State extends State<Page1> {
  @override
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(
        backgroundColor: Theme.of(context).colorScheme.inversePrimary,
        title: const Text('Page2'),
      ),
      body: Center(
        child: Column(
          children: [
            TextButton(
              onPressed: () {
                Navigator.of(context).push(
                    MaterialPageRoute(builder: (_) => const Page2()));
              },
              child: const Text('to web page'),
            ),
          ],
        ),
      ),
    );
  }
}


class Page2 extends StatefulWidget {

  const Page2({super.key});

  @override
  State<StatefulWidget> createState() => _Page2State();
}

class _Page2State extends State<Page2> {
  late WebViewController _controller;
  bool _canPop = false;

  @override
  void initState() {
    super.initState();
    late final PlatformWebViewControllerCreationParams params;
    if (WebViewPlatform.instance is WebKitWebViewPlatform) {
      params = WebKitWebViewControllerCreationParams(
        allowsInlineMediaPlayback: true,
        mediaTypesRequiringUserAction: const <PlaybackMediaTypes>{},
      );
    } else {
      params = const PlatformWebViewControllerCreationParams();
    }

    final WebViewController controller =
    WebViewController.fromPlatformCreationParams(params);
    controller
      ..setJavaScriptMode(JavaScriptMode.unrestricted)
      ..setBackgroundColor(const Color(0x00000000))
      ..setNavigationDelegate(
        NavigationDelegate(
          onProgress: (int progress) {},
          onPageStarted: (String url) {},
          onPageFinished: (String url) {},
          onWebResourceError: (WebResourceError error) {},
          onNavigationRequest: (NavigationRequest request) =>
          NavigationDecision.navigate,
          onUrlChange: (UrlChange change) {},
          // 🔥🔥🔥 This is called when the underlying web view changes to a new route.
          // onCanGoBackChange: (canGoBack){
          //  setState(() {
          //    _canPop = canGoBack;
          //  });
          // }
        ),
      )
      ..loadRequest(Uri.parse('https://docs.flutter.dev/release/release-notes'));

    if (controller.platform is AndroidWebViewController) {
      AndroidWebViewController.enableDebugging(true);
      (controller.platform as AndroidWebViewController)
          .setMediaPlaybackRequiresUserGesture(false);
    } else if (controller.platform is WebKitWebViewController) {
      // 🔥🔥🔥 Enable the swipe-back gesture on iOS devices.
      (controller.platform as WebKitWebViewController)
          .setAllowsBackForwardNavigationGestures(true);
    }
    _controller = controller;
  }

  @override
  Widget build(BuildContext context) {
    return PopScope(
      // 🔥🔥🔥 If it's not the initial H5 page, disable the swipe-back gesture,
      // but allow returning to the parent H5 page.
      canPop: !_canPop,
      onPopInvokedWithResult: _canPop ? (_, __) => Future.value(true) : null,
      child: Scaffold(
        appBar: AppBar(
          backgroundColor: Theme.of(context).colorScheme.inversePrimary,
          title: Text('canGoBack: $_canPop'),
          actions: [
            IconButton(
              onPressed: () async {
                _canPop = await _controller.canGoBack();
                print("canGoBack: $_canPop");
                setState(() {});
              },
              icon: const Icon(Icons.update),
            ),
          ],
        ),
        body: WebViewWidget(controller: _controller),
      ),
    );
  }
}

@github-actions github-actions bot removed the waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds label Nov 6, 2024
@StanleyCocos
Copy link
Contributor Author

Explanation of the iOS swipe-to-return issue.: #14203

@huycozy huycozy added c: new feature Nothing broken; request for a new capability p: webview The WebView plugin package flutter/packages repository. See also p: labels. c: proposal A detailed proposal for a change to Flutter team-ecosystem Owned by Ecosystem team and removed in triage Presently being triaged by the triage team labels Nov 7, 2024
@StanleyCocos
Copy link
Contributor Author

@huycozy Thank you.
I'm not entirely sure about the specific process.
I'd like to ask if this stage means that the feature has already been approved. If it has, can I accept the task and create a PR request?

@huycozy
Copy link
Member

huycozy commented Nov 7, 2024

The issue is forwarded to ecosystem team (team manages 1st party packages) to triage (please check Triage process for teams). So please wait response from ecosystem team to decide whether the feature should be added or not.

@stuartmorgan
Copy link
Contributor

@StanleyCocos Could you just check canGoBack in the navigation delegate when the URL changes? Ideally we wouldn't add new callbacks for every property, and I would not expect this one to change without an existing delegate call also firing.

@stuartmorgan stuartmorgan added the waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds label Nov 12, 2024
@StanleyCocos
Copy link
Contributor Author

Apologies, I don't fully understand your point. Regarding the issue I'm currently facing, some actions that may occur in H5 only trigger changes in the history, but do not invoke the callbacks of the existing NavigationDelegate methods. Therefore, adding onCanGoBackChange seems necessary.

@github-actions github-actions bot removed the waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds label Nov 13, 2024
@stuartmorgan
Copy link
Contributor

some actions that may occur in H5 only trigger changes in the history, but do not invoke the callbacks of the existing NavigationDelegate methods

Could you give specific examples? It would be good to understand whether we are missing important navigation delegate calls.

Therefore, adding onCanGoBackChange seems necessary.

The fact that the current behavior of the navigation delegate doesn't cover all use cases doesn't necessarily mean that adding this specific callback is the only (or best) solution.

You mentioned in the original report that you would like to send a PR for this. What would the Android implementation of that PR be? It's not clear to me how this would be done on Android other than by checking the state in other delegate methods, at which point exposing those delegate methods would be a better solution.

@StanleyCocos
Copy link
Contributor Author

Apologies, you're right; this can't be achieved effectively on Android. I've tried many methods over the past few days. I might need to find another approach to achieve this. Currently, it seems my suggestion is not quite suitable. Do you have any other advice?

@stuartmorgan
Copy link
Contributor

Given that we can only support this on iOS, I'm not sure it would be valuable to implement currently.

@stuartmorgan stuartmorgan added P3 Issues that are less important to the Flutter project triaged-ecosystem Triaged by Ecosystem team labels Nov 20, 2024
@StanleyCocos
Copy link
Contributor Author

Because I encountered this issue and wanted to add the onCanGoBackChanged property, its primary purpose is to address the swipe-back issue in the iOS ecosystem. I have explained above why this approach is necessary for resolving the swipe-back problem. Therefore, I believe implementing this property specifically to address the iOS ecosystem's issue seems reasonable, and limiting it to the iOS ecosystem appears to make sense.

@lnd1992

This comment was marked as off-topic.

@StanleyCocos
Copy link
Contributor Author

@stuartmorgan Hello
What are your thoughts on this feature request? I can implement this functionality for the iOS version, but due to Android's limitations, I cannot implement it as effectively there. I'm unsure whether this request can still proceed with a valid PR. Could you guide me on this? If it can still be done, what plans do you have for the Android implementation?

@stuartmorgan
Copy link
Contributor

I'm unsure whether this request can still proceed with a valid PR.

It could be added as a platform-specific API that's exposed at the webview_flutter_wkwebview level but not the webview_flutter level.

@StanleyCocos
Copy link
Contributor Author

Got it, I understand. I'll make some changes and need a bit of time. I expect to submit the feature request next week.

@StanleyCocos
Copy link
Contributor Author

Hello @stuartmorgan ,
as per your request, I have added the new changes at the webview_flutter_wkwebview level. If further adjustments are needed, I will do my best to modify and follow up. I hope the changes can be merged soon.
Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: new feature Nothing broken; request for a new capability c: proposal A detailed proposal for a change to Flutter p: webview The WebView plugin P3 Issues that are less important to the Flutter project package flutter/packages repository. See also p: labels. team-ecosystem Owned by Ecosystem team triaged-ecosystem Triaged by Ecosystem team
Projects
None yet
4 participants