React-native-orientation: Road to 2.0

[andrerfneves]

Hey all. I understand a lot of us rely on this package to deal with orientation-specific capabilities within our React Native apps, that includes me. Along with a couple of other maintainers, we're going to do a triage of the PRs and the Issues at hand in the repo and make a plan accordingly. Please bear with us as we bring this repo up back up to speed.

Please help us identify the major pain points that need to be fixed and updated by answering below. Although all that stuff is available on the respective Issues, it'd help us a lot to identify them.

Current points we are looking at:

• [X] RN 0.40+ Update for iOS compatibility - 5a16ac8a3f96e12969d86a1dd1f6d548efc213b2
• [x] Remove unncessary (check) Bluetooth permissions requirement
• [x] Publish new version (2.0) to npm

Feature Requests:

If you have a feature request, post on this thread and we can discuss whether implementation should be within the scope of this package.

[colinramsay]

re: point 1, there are at least seven PRs open to solve this. The latest is from 5 days ago (#172), so maybe we just pick that one and close the others.

138 is designed to fix the Bluetooth issue.

What's the branching & release strategy to follow? Maybe just push a quick 2.0 and decide after.

[andrerfneves]

@colinramsay I say we round out all the PRs that need to get merged / tested, merge those/close the others and then release a 2.0. We should release the 2.0 when the major points are addressed.

In regards to #172, that would cause the package to not be backwards compatible, so maybe #165 is a better choice. I also have to test #138 as I'm not sure what the deal with Bluetooth Permissions are. Have you tested those?

After releasing a 2.0, we should tag it as a release, write a changelog and then let's move master branch to only allow for commits through PRs. That way we work off of develop branch, and have PRs based off of develop. It would be a smooth flow. I can take care of Releases/Changelog, unless someone else wants to do that.

@colinramsay what are your thoughts?
I went ahead and merged #165.

[stoneman1]

@andrerfneves I think your way to go is the correct. Only allow through PR to master and work off from develop. You can take care of the releases/changelog if you have the time 👍

There was some interesting things here: #154 that fixes some crash thing? Anyone else had problems with crashing? Should we fix those?

[andrerfneves]

@stoneman1 I haven't experienced any crashes myself. Do you mind taking a look at #154 and see if you find anything? I'll continue the triage of the issues and trying to resolve/close the PRs.

[stoneman1]

@andrerfneves Yeah will check that out

[colinramsay]

@andrerfneves yes, that approach seems good. You seem to be doing a good job leading the charge so I'll just skulk around doing minor triage, but if you need anything specific let me know 👍

[andrerfneves]

Thanks @stoneman1. @colinramsay yeah that sounds good to me. I'll keep going through each of these issues. If you have any time to go through a couple of them and respond/ask for details/debug and at least add the correct labels that would be great. That way we know which are actual bugs and/or just issues from the user's codebase.

[bharath-kamath]

Please look to get android working, as its not working on Android 6.0.1.

[stoneman1]

@bharath-kamath What is the error? Is there a issue about it? It probably has something to do with the bluetooth permissions and location services

[bharath-kamath]

On master, I get the error mentioned in #160

When I try with the npm version, I am getting no errors, but then the listeners do not fire when I change orientation.

[stoneman1]

@andrerfneves @colinramsay should we use squash and merge only? I think it is the best way to keep the repo clean :)

[andrerfneves]

As long as we make sure the final PR commit that gets merged is descriptive enough for the feature/fix being implemented, I'm all for squashing and merging @stoneman1.

@bharath-kamath I'm going to have some time to look at this stuff today.

[NikitaSergeevich]

Hi guys, this one is urgent for me. Please take a look. Thanks for awesome library!

[rBurgett]

I'm still getting the error from #114. Is that being addressed? The app won't build on Android. I am on an extremely tight schedule w/ my client and I can't finish this app if it won't even run on Android. Thanks for an otherwise excellent library!

[andrerfneves]

@rBurgett what version of RN are you using?

[rBurgett]

@andrerfneves 0.44.2

[andrerfneves]

@rBurgett weird I'm using this in two applications on 0.44.1 right now. Both iOS and Android work well. What's your MainApplication and MainActivity look like?

[rBurgett]

Here you go, @andrerfneves.

MainActivity.java

package com.trafero;

import com.facebook.react.ReactActivity;

import android.content.Intent;
import android.content.res.Configuration;

public class MainActivity extends ReactActivity {

    @Override
    public void onConfigurationChanged(Configuration newConfig) {
        super.onConfigurationChanged(newConfig);
        Intent intent = new Intent("onConfigurationChanged");
        intent.putExtra("newConfig", newConfig);
        this.sendBroadcast(intent);
    }

    /**
     * Returns the name of the main component registered from JavaScript.
     * This is used to schedule rendering of the component.
     */
    @Override
    protected String getMainComponentName() {
        return "trafero";
    }
}

MainApplication.java

package com.trafero;

import android.app.Application;

import com.facebook.react.ReactApplication;
import com.github.yamill.orientation.OrientationPackage;
import com.oblador.vectoricons.VectorIconsPackage;
import com.facebook.react.ReactNativeHost;
import com.facebook.react.ReactPackage;
import com.facebook.react.shell.MainReactPackage;
import com.facebook.soloader.SoLoader;
import java.util.Arrays;
import java.util.List;

public class MainApplication extends Application implements ReactApplication {
  private final ReactNativeHost mReactNativeHost = new ReactNativeHost(this) {
    @Override
    public boolean getUseDeveloperSupport() {
      return BuildConfig.DEBUG;
    }
    @Override
    protected List<ReactPackage> getPackages() {
      return Arrays.<ReactPackage>asList(
          new MainReactPackage(),
            new OrientationPackage(),
            new OrientationPackage(this),
            new VectorIconsPackage()
      );
    }
  };
  @Override
  public ReactNativeHost getReactNativeHost() {
    return mReactNativeHost;
  }
  @Override
  public void onCreate() {
    super.onCreate();
    SoLoader.init(this, /* native exopackage */ false);
  }
}

[andrerfneves]

@rBurgett so removing this from here and also remove the duplicate new OrientationPackage() should get it to build.

 protected List<ReactPackage> getPackages() {
      return Arrays.<ReactPackage>asList(
          new MainReactPackage(),
          new OrientationPackage(), // no more this here
          new VectorIconsPackage()
      );
    }

[rBurgett]

That's fantastic, @andrerfneves! I can't thank you enough for the quick advice. It's working perfectly. So, was the problem caused by react-native link?

[andrerfneves]

I believe you might have first run react-native link with the outdated npmjs version of the package. That version uses an old constructor in the Android implementation which is different than the version in master branch user here. Thus the difference in syntax. Not only that, but if you ran react-native link after switching to the new package you would have gotten a double import statement, like you did with:

new OrientationPackage(),
new OrientationPackage(this),

Either way, on fresh installations through master this should work for iOS and Android and react-native link.

Glad I could help @rBurgett

[kesha-antonov]

Heys guys!

Anybody knows how to get orientation for android when orientation locked? (It works in iOS)

https://github.com/yamill/react-native-orientation/pull/106#issuecomment-309106451

[genaby]

@kesha-antonov
in degrees 0, 90, 180, 270:

   orientationListener = new OrientationEventListener(this, SensorManager.SENSOR_DELAY_NORMAL) {      
      @Override
      public void onOrientationChanged(int orientation) {
        int rounded = (orientation + rotation * 90 + 44) / 90;
        rounded &= 3;
        rounded *= 90;
 ...
      }
    };

enable:

    if(orientationListener != null && orientationListener.canDetectOrientation()) 
        orientationListener.enable();

disable

    if(orientationListener != null && orientationListener.canDetectOrientation()) 
        orientationListener.disable();

[genaby]

@kesha-antonov
forgot rotation

    final int rotation = ((WindowManager)getSystemService(WINDOW_SERVICE)).getDefaultDisplay().getRotation();

[kesha-antonov]

@genaby Thanks!

[StefanWallin]

I just want to chime in that I've experienced the crash mentioned in https://github.com/yamill/react-native-orientation/pull/154 and have made a short-term fork of the master branch for my company based on that issue alone. Look forward to this project getting back on track.

I experience the crash on using the hardware back button to exit the app on Android 6.0.1 on a Samsung Galaxy S5 Neo. Currently in the process of upgrade to using React Native 0.44 and this was one of my roadblocks.

[andrerfneves]

I'm closing this as the issues pointed above don't relate to the 2.0 release issue. Please open a corresponding issue if it still persists.