Closed RaulCorreia closed 6 years ago
Hey @RaulCorreia thank you for using the library and glad to hear you're enjoying it :)
Can you please provide more specific details?
Do you have any logs (builds logs Android logcat logs you can provide)?
Can you setup an example repo that reproduces this issue?
Thanks :)
I was using getSetting to start the state and expose the user that the gps was already on, and I used the listening to update the real time if I disconnect the gps, but only worked all these functions until the android 6, from the 7 none action works, it does not even get done.
I believe that some native library that you are using, from the android 7 should not work anymore. excuse my english
Hey @RaulCorreia I'll try to setup a test environment and look into it during the following week
Hey @RaulCorreia I just updated our example to a new version of react native and tested it on both a Nexus 5X emulator with Android 7.1 and a Nexus 4 Physical device with Lineage OS 15.1 (Android 8.1.0). Everything seems to work properly (getting settings and listening to events). Can you please make sure you can make the example here work: https://github.com/rmrs/react-native-settings/tree/master/example
If that works for you, we can compare the differences between your code and the example code to find the problem.
Also, it would make it easier if you setup a github repo with your own example reproducing the bug.
i use your exemple but an interesting thing that I noticed, when I click on change, and go to the settings and change it, the library detects, but if I change by the action bar does not change. I tested the emulator and the cell phone with api 24. but when I do the same on a cell phone with api 23, the library works perfectly
an emulator image with the location off http://prntscr.com/l7m00w
PS: I tested the airplane mode it happens the same
@RaulCorreia thank you again for pointing out this problem to us. Do you mind trying with this branch: https://github.com/rmrs/react-native-settings/tree/fix_example_event_listeners/example
It looks like one needs to register the receivers explicitly now. If it works for you I'll update the readme as well as the example.
Thanks
@erezrokah just tested on emulator (genymotion) and real device (all api 26):
My suggestions:
RNSettingsModule
constructor) from user MainApplication
Or even it may be better to add new method for dynamic registration:
const listener = RNSettings.onChange(RNSettings.LOCATION_SETTING, e => ...);
...
listener.remove();
And register receiver inside onChange
in java if it was not registered before. And remove receiver if no listeners left.
@erezrokah using this last example in api 27 and 24 in the emulator worked everything correctly, I'll test tomorrow on the cell phones, thanks for the quick concert
@farwayer and @RaulCorreia thank you for verifying.
@farwayer As listening to events should be optional I'm not sure we should put it in the constructor. I'll update the readme and add the new way to the changelog (as you suggested) for now.
I'm still not sure about adding a new method as one needs to register the event listeners using react native api anyway, so it might be confusing.
Since changing the readme+changelog is the easiest and fastest (no code changes required in the library) let's see if it is enough at the moment.
Fixed by https://github.com/rmrs/react-native-settings/pull/24 Please re-open if you have any more issues
IMO it is better avoid manual configuration and code modification. Anyway thanks!
I can give you example how I realized such behaviour with receiver auto-register/unregister in one of my modules.
Module index.js
:
import {NativeModules, NativeEventEmitter} from 'react-native'
const {Wifier} = NativeModules;
class Emitter extends NativeEventEmitter {
_listenerCounts = {};
constructor() {
super(Wifier);
}
addListener(event, ...args) {
const count = this._listenerCounts[event] || 0;
if (!count) Wifier.startListen(event);
this._listenerCounts[event] = count + 1;
return super.addListener(event, ...args);
}
removeAllListeners(event, ...args) {
super.removeAllListeners(event, ...args);
const count = this._listenerCounts[event] || 0;
if (count) Wifier.stopListen(event);
this._listenerCounts[event] = 0;
}
removeSubscription(subscription) {
super.removeSubscription(subscription);
const event = subscription.eventType;
const count = this._listenerCounts[event] || 0;
if (!count) return;
if (--count === 0) Wifier.stopListen(event);
if (count < 0) count = 0;
this._listenerCounts[event] = count;
}
}
const emmiter = new Emitter();
Wifier.addListener = emmiter.addListener.bind(emmiter);
export default Wifier;
In native code:
startListen(event)
register receiver and stopListen(event)
unregister.
And from user code:
this._networksListener = Wifier.addListener(Wifier.Events.NetworksUpdated, networks => ...);
...
this._networksListener.remove();
This is simular how Keyboard
from React Native do.
Hey @farwayer, sorry for the late reply, I wanted to read more about NativeEventEmitter
.
React native's documentation is a bit confusing as the Android docs use DeviceEventEmitter
directly:
https://facebook.github.io/react-native/docs/native-modules-android
and the iOS docs wrap it with NativeEventEmitter
:
https://facebook.github.io/react-native/docs/native-modules-ios
I agree it make sense to extend NativeEventEmitter
as you suggested.
Would you like to submit a pr for it?
@erezrokah yes docs is a bit confusing. I spent several hours reading React Native source to understand how things work. I am quite busy this days. Will provide PR when I will have some free time. Thanks for useful lib!
I really enjoyed using this library, I was testing on an android with api 23 and it worked very well, but when testing on an android from api 24 it did not work anymore. would that be it? or is there anything I can do to make it work on android 24+