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

React Native New Architecture support Done #850

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

SolankiYogesh
Copy link

feat: added fabric example

fix: example testing

fix: removed unwated scripts

feat: new architecture support done

feat: added fabric example

fix: example testing

fix: removed unwated scripts

feat: new architecture support done
SolankiYogesh and others added 4 commits November 22, 2024 18:07
Delete sound.ts

Delete package-lock.json

Update RNSound.podspec

Update RNSound.podspec

Update RNSound.podspec

Update RNSound.podspec

Update package.json
@RomualdPercereau
Copy link
Collaborator

Thank you @SolankiYogesh. I don't currently have time to review & test. If anyone can review it? :)

@Okelm
Copy link

Okelm commented Nov 26, 2024

Thanks @SolankiYogesh !
Do you use this in production at the moment?

These methods are missing for Android.

@ReactMethod
 public void enable(final Boolean enabled) {
   // no op
 }

 @Override
 public Map<String, Object> getConstants() {
   final Map<String, Object> constants = new HashMap<>();
   constants.put("IsAndroid", true);
   return constants;
 }

enable seems to be just a stub,
getConstants might cause some regression for some devs

For iOS while there's enableInSilenceMode method, enable is missing.

enable vs enableInSilenceMode - these serve different purposes in iOS:

  • enable: Uses AVAudioSessionCategoryAmbient - sound is muted when phone is in silent mode
  • enableInSilenceMode: Uses AVAudioSessionCategoryPlayback - sound plays even in silent mode

@RomualdPercereau maybe let's consider releasing alpha version with the new arch and ask devs to test it with us. I'll try to run some tests as well

@SolankiYogesh
Copy link
Author

@Okelm i think both methods are not required because. enable is for IOS only and for IsAndroid i am directly using Platfrom.OS

ok let me check again all code and yes i am currently using this patch in my product app

@SolankiYogesh
Copy link
Author

ok yes enable method is missing let me add

`
RCT_EXPORT_METHOD(enable : (BOOL)enabled) {
AVAudioSession *session = [AVAudioSession sharedInstance];
[session setCategory:AVAudioSessionCategoryAmbient error:nil];
[session setActive:enabled error:nil];
}

`

@Okelm
Copy link

Okelm commented Nov 26, 2024

As long as there's no plan to ditch Windows support this will require Windows new arch rewrite as well.

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.

3 participants