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

fix(dynamic_links): not working on app start (#100) #255

Closed
wants to merge 1 commit into from

Conversation

jherencia
Copy link
Contributor

Description

People have posted some issues:

I've based my work on the release of rn-firebase 6.0 that had the same problems and were fixed so this PR takes some ideas from them.

Related Issues

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • If the pull request affects only one plugin, the PR title starts with the name of the plugin in brackets (e.g. [cloud_firestore])
  • My PR includes unit or integration tests for all changed/updated/fixed behaviors (See Contributor Guide).
  • All existing and new tests are passing.
  • I updated/added relevant documentation (doc comments with ///).
  • The analyzer (flutter analyze) does not report any problems on my PR.
  • I read and followed the Flutter Style Guide.
  • The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
  • I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy.
  • I updated CHANGELOG.md to add a description of the change.
  • I signed the CLA.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

  • Yes, this is a breaking change (please indicate a breaking change in CHANGELOG.md and increment major revision).
  • No, this is not a breaking change.

@HofmannZ
Copy link

HofmannZ commented Oct 7, 2019

Tested locally, can confirm this solves #100.

@quetool
Copy link

quetool commented Oct 9, 2019

@jherencia I can confirm your PR fix the problem on iOS when app is in "killed state" on iOS 13. Thanks!

@jherencia jherencia force-pushed the firebase_dynamic_links branch 2 times, most recently from 0815af3 to 6a3cbbe Compare October 9, 2019 18:53
@quetool
Copy link

quetool commented Oct 9, 2019

@jherencia what is the status of this PR?

@quetool
Copy link

quetool commented Oct 9, 2019

I dont know why but I needed to add a delay of 700ms in order to get deepLink.queryParameters

final PendingDynamicLinkData data = await FirebaseDynamicLinks.instance.getInitialLink();
final Uri deepLink = data?.link;

if (deepLink != null) {
  Future.delayed(Duration(milliseconds: 700), () {
    print(deepLink.queryParameters)
  });
}

FirebaseDynamicLinks.instance.onLink(onSuccess: (PendingDynamicLinkData dynamicLink) async {
  final Uri deepLink = dynamicLink?.link;

  if (deepLink != null) {
    Future.delayed(Duration(milliseconds: 700), () {
      print(deepLink.queryParameters)
    });
  }
}, onError: (OnLinkErrorException e) async {
  print('onLinkError ' + e.toString());
});

@jherencia
Copy link
Contributor Author

@jherencia what is the status of this PR?

Do not know, it is in Google's hands.

I guess @bparrishMines could give us more info.

@jherencia
Copy link
Contributor Author

I dont know why but I needed to add a delay of 700ms in order to get deepLink.queryParameters

final PendingDynamicLinkData data = await FirebaseDynamicLinks.instance.getInitialLink();
final Uri deepLink = data?.link;

if (deepLink != null) {
  Future.delayed(Duration(milliseconds: 700), () {
    print(deepLink.queryParameters)
  });
}

FirebaseDynamicLinks.instance.onLink(onSuccess: (PendingDynamicLinkData dynamicLink) async {
  final Uri deepLink = dynamicLink?.link;

  if (deepLink != null) {
    Future.delayed(Duration(milliseconds: 700), () {
      print(deepLink.queryParameters)
    });
  }
}, onError: (OnLinkErrorException e) async {
  print('onLinkError ' + e.toString());
});

I'm going to check this.

@jherencia
Copy link
Contributor Author

@quetool

I've tried and everything work OK without the delay, this is my demo app:

import 'package:flutter/material.dart';
import 'package:firebase_dynamic_links/firebase_dynamic_links.dart';

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

class MyApp extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      title: 'Flutter Demo',
      theme: ThemeData(
        primarySwatch: Colors.blue,
      ),
      home: MyHomePage(title: 'Flutter Demo Home Page'),
    );
  }
}

class MyHomePage extends StatefulWidget {
  MyHomePage({Key key, this.title}) : super(key: key);

  final String title;

  @override
  _MyHomePageState createState() => _MyHomePageState();
}

class _MyHomePageState extends State<MyHomePage> {
  GlobalKey<ScaffoldState> scaffoldKey = new GlobalKey<ScaffoldState>();

  @override
  void initState() {
    super.initState();
    this.initDynamicLinks();
  }

  void showMessage(String message, { Color backgroundColor = Colors.green}) {
    this.scaffoldKey.currentState.showSnackBar(SnackBar(
      backgroundColor: backgroundColor,
      content: Text(message)
    ));
  }

  void initDynamicLinks() async {
    final PendingDynamicLinkData data = await FirebaseDynamicLinks.instance.getInitialLink();
    final Uri deepLink = data?.link;

    if (deepLink != null) {
      this.showMessage('onInitialLink: $deepLink');
    }

    FirebaseDynamicLinks.instance.onLink(
      onSuccess: (PendingDynamicLinkData dynamicLink) async {
        final Uri deepLink = dynamicLink?.link;

        if (deepLink != null) {
          this.showMessage('onLinkSuccess $deepLink');
        }
        else {
          this.showMessage('onLinkSuccess empty deepLink', backgroundColor: Colors.orange);
        }
      },
      onError: (OnLinkErrorException e) async {
        this.showMessage('onLinkError ${e.message}', backgroundColor: Colors.red);
      }
    );
  }

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      key: scaffoldKey,
      appBar: AppBar(
        title: Text(widget.title),
      ),
      body: Center(
        child: Column(
          mainAxisAlignment: MainAxisAlignment.center,
          children: <Widget>[
            Text(
              'This is an examle of dynamic links, open a dynamic link and you will see the snackBar showing the link',
            ),
          ],
        ),
      ),
    );
  }
}

Can you check with this code?

@quetool
Copy link

quetool commented Oct 10, 2019

@jherencia yes, sorry! I was making a mistake on my code! Everything is working fine without the delay! Thanks!

@quetool
Copy link

quetool commented Oct 10, 2019

@jherencia is it possible your PR is not working on iOS when a user install the app from a dynamic link? I can't get anything from FirebaseDynamicLinks.instance.getInitialLink()

@JayM96
Copy link

JayM96 commented Oct 22, 2019

@quetool can you please give me how you have implement your dynamic links in flutter app the thread Here

I am stuck. Your help will make my day.

Thanks.

@logemann
Copy link

fighting with dynamicLinks too. how can i test this fix ? Still on latest dev branch.

@JayM96
Copy link

JayM96 commented Oct 22, 2019

I understand. Thanks for your reply.

Still if you can hint if anything I might be doing wrong?

Also, I am using flutter firebase dynamic links plugin. Should I need to integrate your package in my app Or plugin will work properly.

Thanks again.

@quetool
Copy link

quetool commented Oct 22, 2019

@quetool can you please give me how you have implement your dynamic links in flutter app the thread Here

I am stuck. Your help will make my day.

Thanks.

Hi! May this help you? #100 (comment)
Otherwise, can you describe your issue?

@HofmannZ
Copy link

@bparrishMines Any thoughts?

@HofmannZ
Copy link

@collinjackson I have to use this fork for too long now, can we at least get a review?

@jherencia jherencia force-pushed the firebase_dynamic_links branch from c94173c to 3b05296 Compare November 22, 2019 17:01
@jherencia
Copy link
Contributor Author

I've syncronized again the fork and changed the tag so it is independent from the package version which is inconsistent.

  firebase_dynamic_links:
    git:
      url: https://github.com/mrmilu/flutterfire.git
      path: packages/firebase_dynamic_links
      ref: PR-255-v1

@kwent
Copy link

kwent commented Feb 10, 2020

How can we help to move this PR forwards. Been few months... Regards

@jackforesightmobile
Copy link

We're also really interested in getting this PR merged.

@MrAlek
Copy link

MrAlek commented Mar 18, 2020

We're also stuck on this. Another ping to maintainers @bparrishMines @collinjackson

@tapizquent
Copy link

Any update on this??

@koskimas
Copy link

koskimas commented Apr 28, 2020

This is pretty critical for many apps. Without this, there's no way to get the link that opened the app on iOS. Doesn't this bug partially defeat the whole purpose of this library? I'd love to see this merged soon too!

@tapizquent
Copy link

This is pretty critical for many apps. Without this, there's no way to get the link that opened the app on iOS. Doesn't this bug partially defeat the whole purpose of this library? I'd love to see this merged soon too!

I had to fork this library and implement the fixes myself. Which kinda sucks, as I will have to be paying attention to any breaking changes and re implement the fix.

@m-j-g
Copy link

m-j-g commented May 7, 2020

This is pretty critical for many apps. Without this, there's no way to get the link that opened the app on iOS. Doesn't this bug partially defeat the whole purpose of this library? I'd love to see this merged soon too!

I had to fork this library and implement the fixes myself. Which kinda sucks, as I will have to be paying attention to any breaking changes and re implement the fix.

Yeah, at this point it is getting pretty ridiculous how inefficient this is being run.

edit: its been 8 months by the way.

@HofmannZ
Copy link

HofmannZ commented Jun 7, 2020

Another month has passed, been using a fork for 9 months now, this is absurd... I understand that is open source so time and resources are limited, but this PR has the fix for 9 months now... Just merge already.

@theneshofficial
Copy link

theneshofficial commented Jun 7, 2020

@bparrishMines please merge it, not sure what’s holding back since all checks are passed.

@long1eu
Copy link
Contributor

long1eu commented Sep 27, 2020

Why is this not merged?

@jherencia
Copy link
Contributor Author

jherencia commented Oct 9, 2020

Hi all,

I see movement here and in #100 asking why this is not merged.

It's been a year since I created the PR and we do not know what are the plans of Google about this.

I understand that the codebase has changed significantly in this time so the PR probably need some work. I can work on this if we have a clear path to get this merged.

@bparrishMines @collinjackson can someone give us some light?

@Salakar
Copy link
Member

Salakar commented Nov 11, 2020

@jherencia - FlutterFire maintainer here, and oddly enough the author of the RNFirebase v6 release 😆

Would love to have this merged to fix these issues - would you be able to update it, or if possible for better visibility send a new PR (and close this one) and tag me in it - I can then get it merged and shipped ASAP.

@jherencia jherencia force-pushed the firebase_dynamic_links branch from 3b05296 to 936bbed Compare December 8, 2020 18:57
@jherencia jherencia changed the title [firebase_dynamic_links] Fixes several issues on iOS (#100) fix(dynamic_links): Not working on app start (#100) Dec 8, 2020
@jherencia jherencia changed the title fix(dynamic_links): Not working on app start (#100) fix(dynamic_links): not working on app start (#100) Dec 8, 2020
@jherencia jherencia force-pushed the firebase_dynamic_links branch from 936bbed to ed2cda4 Compare December 8, 2020 19:02
@jherencia jherencia force-pushed the firebase_dynamic_links branch from ed2cda4 to 9052f6a Compare December 8, 2020 19:15
@jherencia
Copy link
Contributor Author

@jherencia - FlutterFire maintainer here, and oddly enough the author of the RNFirebase v6 release 😆

Would love to have this merged to fix these issues - would you be able to update it, or if possible for better visibility send a new PR (and close this one) and tag me in it - I can then get it merged and shipped ASAP.

Hi @Salakar,

Ok, I've created a new PR as you suggested:
#4354

@jherencia jherencia closed this Dec 8, 2020
@jherencia jherencia deleted the firebase_dynamic_links branch December 8, 2020 19:36
@firebase firebase locked and limited conversation to collaborators Jan 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.