Skip to content

Proof of concept to handle issue with JS not initialized on start to fix #107 #169

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

Closed

Conversation

konradkierus
Copy link
Contributor

Proof of concept to handle issue with JS not initialized on start to fix #107

Described in details here: #107 (comment)

Example usage:

import { each, compact, last, get } from 'lodash';

RNCallKeep.addEventListener(
  'didLoadWithEvents',
  this.onDidLoadWithNativeEvents
);

onDidLoadWithNativeEvents(events) {
  const validEvents = compact(events);
  let callDataToAdd = null;
  let callDataToAnswer = null;
  let callDataToReject = null;
  let callDataToInitiateCall = null;
  each(compact(validEvents), event => {
    const { name, data } = event;
    if (name === 'RNCallKeepDidDisplayIncomingCall') {
      callDataToAdd = data;
      callDataToAnswer = null;
      callDataToReject = null;
    }
    if (name === 'RNCallKeepPerformAnswerCallAction') {
      callDataToReject = null;
      callDataToAnswer = data;
    }
    if (name === 'RNCallKeepPerformEndCallAction') {
      callDataToAnswer = null;
      callDataToReject = data;
    }
  });

  const lastEventName = get(last(validEvents), 'name');
  if (lastEventName === 'RNCallKeepDidReceiveStartCallAction') {
    callDataToInitiateCall = get(last(validEvents), 'data');
    callDataToAnswer = null;
    if (!callDataToReject) {
      callDataToAdd = null;
    }
  }

  if (callDataToAdd) {
    this.onDisplayIncomingCall(callDataToAdd);
    if (callDataToReject) {
      this.onEndCall(callDataToReject);
    }
    if (callDataToAnswer) {
      this.onAnswerCall(callDataToAnswer);
    }
  }
  if (callDataToInitiateCall) {
    this.onStartCall(callDataToAnswer);
  }
}

@manuquentin
Copy link
Contributor

manuquentin commented Mar 12, 2020

Thanks @konradkierus I'll check it. Can you also update the documentation ?

@konradkierus
Copy link
Contributor Author

@manuquentin just did, feel free to modify if you think it needs more details

@zxcpoiu
Copy link
Member

zxcpoiu commented Mar 13, 2020

@konradkierus
Thanks for your work 👍

I have some thoughts for open discussion.

The principle is: event order is important.
And we hope everything happens behind the scene, so user won't aware and don't have to do additional things.

Based on your concept, how about:

  1. When RN Bridge did not up, queue up events ( the same )
  2. When user setup all required event listener on JS Side, call a function ( like RNCallKeep.ready() )
    ( or at some point, we can call this.ready() somewhere in RNCallKeep internal at JS side? the module constructor? )
  3. In Native Side ready() function, flush and fire all queued events ( which is in order ), then set hasListener = YES ( or just ready = YES )

We may need to give user the last event, just return the last event in ready() function or store in a variable, then let user to getLastEvent()

should discuss:

  • Should events have to expire?
  • Should some events override each other? like if EndCall first, then AsnwerCall the second event in the same uuid:
    • should we only store EndCall and ignore AnswerCall
    • or should we only store AnswerCall and remove EndCall event
    • or should we just send two events in order

@konradkierus
Copy link
Contributor Author

@zxcpoiu

I don't see issues with implementing didLoadWithEvents event internally in the JS part of the module and then call ready() or something similar.
Unfortunately, I don't currently work on the project which was using this approach so I won't be able to test any further changes I would do.

Now, regarding your questions. When I've started thinking about solution for the #107 issue, I was asking myself very similar questions which you are now asking me.
I've decided that it's very hard to implement one and correct solution for everyone as people may have different requirements. Some people (like me) would like to have full control on events order. Other people may be interested about last event only. That's why I've implemented it in this way so anyone could decide by themselves.

@zxcpoiu
Copy link
Member

zxcpoiu commented Mar 14, 2020

Agreed, it's hard to implement a solution suit well for all cases.

And yes, if the js part called internally in js module side, the user won't bother much. That's my whole point try to avoid add additional handling.

That's wait others thoughts and different use cars.

Thanks for your elaboration.

@manuquentin
Copy link
Contributor

Hi @konradkierus,
I've created the #205 PR with your work and a small addition to re-emit these event when the bridge is up.

Can you give it a try ?

Thanks!

@konradkierus
Copy link
Contributor Author

Hi @manuquentin
Unfortunately, I"m not currently working on any project which is using this approach and I have limited access to iOS device so I won't be able to test your changes.

@sboily
Copy link
Member

sboily commented Jun 4, 2020

Can we close it and continue the discussion on #205?

@manuquentin
Copy link
Contributor

Closed in favor of #205

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[iOS 13] callKit displayed from AppDelegate.m before incoming call listeners
4 participants