discourse-mobile-single-site-app icon indicating copy to clipboard operation
discourse-mobile-single-site-app copied to clipboard

This needs major updating to be usable

Open coryjreid opened this issue 5 years ago • 19 comments

It appears a lot has changed since this app was cobbled together. OAuth login flows are totally broken in the app. It can't even login with an email or password. And seeing all that will be a miracle because you have to fix things such as following the instructions here to even get the thing building properly.

The README should be updated to make this clear -or- the project updated.

Thanks, though!

coryjreid avatar May 16 '20 19:05 coryjreid

For most oauth providers, logins should work, but you need to whitelist the correct URLs so the authentication happens in the webview. See https://github.com/pmusaraj/discourse-mobile-single-site-app/blob/master/default.variables.js#L24

If you run into issues when building the app, please post the specific issues. Or, better yet, please contribute fixes (I built this repo 2.5 years ago and have not had to use it a lot recently).

pmusaraj avatar May 17 '20 13:05 pmusaraj

@pmusaraj I'm not going to work on this so soon, but considering their current states do you think the best bet nowadays would be to fork this repo or discourse/DiscourseMobile itself as baseline for a single site app?

Off-topic/Curiosity: I didn't know this repo existed and was set to work on DiscourseMobile, but this topic appeared on meta's /latest ~30min ago, but there's no recent interaction there.

renato avatar Dec 04 '20 15:12 renato

@renato it so happens that I am updating this repo. Might take another few weeks, but it is not dead!

pmusaraj avatar Dec 04 '20 17:12 pmusaraj

How you getting in with the update? I’m intrigued (and super keen to see if push notifications will be fixed!).

afdy avatar Jan 10 '21 21:01 afdy

@renato @afdy I have just merged a major update to the repo. I think this is now in very good shape, please give it a try. Make sure you try it on a fresh clone and using the updated discourse-onesignal plugin.

The repo now includes Fastlane scripts that simplify greatly the task of adapting this whitelisted repo for your own app (and its assets, certificates, naming and configuration). Make sure you review the updated readme file and let me know if there are any mistakes in there.

pmusaraj avatar Jan 20 '21 15:01 pmusaraj

Awesome thanks - just had a blast, couple of minor points so far;

  • new guide doesn't mention creating app.variables.js
  • fastlane swith fails with "[07:36:58]: No such file or directory @ rb_sysopen - EastleighOnline/android-release.keystore" as I wasn't releasing on android. I guess i have no excuse but to make that work now! ;)

I'll continue later!

afdy avatar Jan 22 '21 07:01 afdy

  • new guide doesn't mention creating app.variables.js

Right, because it's one of the items in the sampe folder. Maybe I'll make it more explicit.

  • as I wasn't releasing on android

Ah right, you can probably comment out the android lines in that fastlane task in Fastfile.

Thanks for trying this out.

pmusaraj avatar Jan 22 '21 15:01 pmusaraj

I found another couple of issues;

  • I had to upgrade the OneSignal library to not support UIWebView (against apple guidelines). I upgraded to 3.9.3 to make it work, but there are later versions.
  • I went to attempt android build so generated a keystore (which fixed iOS builds), but then bumped into another error:

I am struggling with where to start here, some brief googling around suggests its due to google library version issues.

Task :app:processReleaseResources FAILED Task :app:bundleReleaseResources FAILED

FAILURE: Build completed with 2 failures.

1: Task failed with an exception.

  • What went wrong: Execution failed for task ':app:processReleaseResources'.

A failure occurred while executing com.android.build.gradle.internal.tasks.Workers$ActionFacade Android resource linking failed AAPT: error: resource style/SplashScreen_SplashTheme (aka com.myapp.app:style/SplashScreen_SplashTheme) not found. error: failed linking references.

afdy avatar Jan 23 '21 10:01 afdy

Retried today from scratch, in case it was the OneSignal upgrade that broke it - but no, same error.

afdy avatar Jan 23 '21 16:01 afdy

Got it - I had to add react-native-splash-screen and then the android version built too. Missing from package.json. That was a hard one to track.

afdy avatar Jan 23 '21 19:01 afdy

Thanks for trying this out and for the feedback. I have made some updates in the main branch:

  • updated OneSignal to 3.93, that's a good idea
  • removed the splashscreen stuff for Android, it wasn't hooked up properly, and in the vast majority of the devices, the app loads fast enough not to need that screen
  • removed another dependency that was no longer being used

pmusaraj avatar Jan 23 '21 20:01 pmusaraj

Cool thanks - just tried a fresh run and it fails with the same error as previous until I add react-native-splash-screen. Fastlane build process must call it somewhere.

afdy avatar Jan 23 '21 21:01 afdy

Hmm... it builds on Android for me without that package. It's possible you have some leftover references from the previous switch command in the Android files.

pmusaraj avatar Jan 23 '21 21:01 pmusaraj

Hmm I'd say not possible, I checked out a whole new directory, only files that persist are ones I've copied over in .env.myapp, and the contents of the myapp folder.

I've just recreated again and it does exactly the same thing.

git log shows I definitely pulled in your commits. :)

I see SplashScreen_SplashTheme is still defined here: https://github.com/pmusaraj/discourse-mobile-single-site-app/blob/80eb9b47636548df58d0745c487fde6e48d3f7ec/android/app/src/main/res/values/styles.xml

afdy avatar Jan 24 '21 09:01 afdy

Removed this bit:

    <style name="SplashScreenTheme" parent="SplashScreen_SplashTheme">
        <item name="colorPrimaryDark">@color/splashprimary</item>
    </style>

from styles.xml and it builds without it now.

afdy avatar Jan 24 '21 10:01 afdy

Indeed, I had left those lines in during the last fix. It's fixed now though.

pmusaraj avatar Jan 24 '21 14:01 pmusaraj

Awesome, this now works great thanks! Awesome work by the way, really useful stuff!

Love fastlane - is there any way fastlane can take over the build numbering in the custom env files? Right now things get up if I pull a clean version again as the build numbers go backwards! :)

afdy avatar Jan 29 '21 16:01 afdy

Love that there is no longer a need to open XCode - how hard would it be to do an iPad optimised version?

afdy avatar Jan 29 '21 16:01 afdy

Yes, those are two good and fairly doable improvements: envfiles controlling the build version and number, and support for iPad. Not sure when I'll get to the them, tbh. PRs are certainly welcome!

pmusaraj avatar Feb 01 '21 14:02 pmusaraj