Skip to content

Upgrade all dependencies and create a new build for iOS#1

Open
madhums wants to merge 18 commits into
masterfrom
feat/cordova-latest
Open

Upgrade all dependencies and create a new build for iOS#1
madhums wants to merge 18 commits into
masterfrom
feat/cordova-latest

Conversation

@madhums

@madhums madhums commented May 1, 2023

Copy link
Copy Markdown
Member

closes https://github.com/q-m/questionmark-web/issues/370

  • prepare for cordova latest
  • use es6 string literal and change LANDING_URL to checker.thequestionmark.org
  • update config to include access and support for iPad
  • update cordova-ios@6.3.0
  • fix regex output

@madhums madhums requested a review from wvengen May 1, 2023 09:55

@wvengen wvengen left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, you made it work ... congratulations!!
Some notes and questions.

Comment thread README.md
Comment thread config.xml Outdated
Comment thread config.xml
Comment thread www/js/index.js Outdated
Comment thread www/js/index.js
Comment thread www/js/index.js
@wvengen

wvengen commented May 1, 2023

Copy link
Copy Markdown
Member

Regarding deployment: Is the app at-risk of being unpublished in the iOS app store? If yes, then we'd better get this published. If not, then let's focus on the Android app first, test and make it work, publish in Google Play, then publish the iOS app later.

@madhums madhums requested a review from wvengen May 1, 2023 10:42
Comment thread README.md
Comment thread README.md Outdated
Comment thread README.md
Comment thread config.xml Outdated
Comment thread www/js/index.js
@madhums

madhums commented May 1, 2023

Copy link
Copy Markdown
Member Author

iOS Distribution Certificate (App Store)
If your Apple Developer Program membership is valid, your existing apps on the App Store won't be affected. However, you'll no longer be able to upload new apps or updates signed with the expired or revoked certificate to the App Store.

From https://developer.apple.com/support/certificates/

I don't think we are at any risk of app being unpublished.

@madhums madhums requested a review from wvengen May 1, 2023 11:03

@wvengen wvengen left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost there!
Now I'm curious how Android is doing :)

Comment thread package.json Outdated
Comment thread www/js/index.js

@wvengen wvengen left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super Madhu, that you managed to get it building again and upgraded Cordova!
One comment about setup, otherwise ready to merge :)

Now I'll have a look at Android, which unfortunately doesn't build yet. I think plugins need to be updated / replaced for that.

Comment thread README.md
npm i
npx cordova requirements # check requirements for building the app
npx cordova platform add ios # only on iOS supported platform
npx cordova platform add android

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest

npm install
npx cordova platform add ios      # when building iOS app on Mac OS
npx cordova platform add android  # when building Android app
npx cordova requirements          # check build requirements
npx cordova build ios             # when building iOS app on Mac OS
npx cordova build android         # when building Android app

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, feel free to do so!

@wvengen

wvengen commented May 2, 2023

Copy link
Copy Markdown
Member

Also, I would really like to make sure everything is tested (and I can only do that on Android), like various ways of navigating, barcode scanning, network online/offline in different conditions.

@madhums

madhums commented May 2, 2023

Copy link
Copy Markdown
Member Author

Yes I am testing too and it seems like there's some issue with custom scheme and scanning doesn't work properly. 😞

@madhums

madhums commented May 2, 2023

Copy link
Copy Markdown
Member Author

The barcode scanner doesn't open and the error seems to be:

2023-05-02 10:53:49.639816+0200 Questionmark[18034:1103463] -canOpenURL: failed for URL: "app://mobile-scan?ret=%2Flookup%3Fapp%3DQuestionmark%26q%3D%7BCODE%7D" - error: "The operation couldn’t be completed. (OSStatus error -10814.)"
2023-05-02 10:54:19.159734+0200 Questionmark[18034:1103463] Could not signal service com.apple.WebKit.WebContent: 113: Could not find specified service

I've tried these config in Info.plist without much luck:

        <key>LSApplicationQueriesSchemes</key>
	<array>
		<string>app</string>
	</array>
	<key>NSAppTransportSecurity</key>
	<dict>
		<key>NSAllowsArbitraryLoads</key>
		<true/>
	</dict>

And have the same issue with opening mailto:* or tel:* links (Similar to apache/cordova-plugin-inappbrowser#830)

Related old PR, perhaps this needs to be merged in order to make it work? @wvengen apache/cordova-plugin-inappbrowser#274

@madhums

madhums commented May 2, 2023

Copy link
Copy Markdown
Member Author

Another note: many of the cordova plugins seem to be unmaintained and haven't been updated since 3-4 years. It may be the end of life for cordova. Something to consider for the future - to switch to a different technology like ReactNative.

@wvengen

wvengen commented May 2, 2023

Copy link
Copy Markdown
Member

I see that many of the current plugins have not been upgraded yet, I'd expect that to be important for a succesful upgrade.
E.g. the barcode reader plugin using the Google ML model hasn't been updated. We can revert to the ordinary barcode scanner.
Some plugins have not been updated in more than a year, but are very much part of the core ecosystem - and may not need an update in this timespan (like inappbrowser). One year of no updates can be perfectly healthy in certain (usually smaller sub-) projects that reached maturity. Not sure if that applies in this case, however.

Something to consider for the future - to switch to a different technology like ReactNative.

That is something we don't have capacity for, as it is basically creating a new front-end. Cordova, or any other embed-website-app-framework, would still be preferred. Ideally, we wouldn't need an app anymore, but use this website-as-app thing (but I don't think barcode scanning has made it yet to browsers - except a slow JS-based version using the webcam that doesn't cut it on lower-end devices at least).

@wvengen

wvengen commented May 2, 2023

Copy link
Copy Markdown
Member

Related old PR, perhaps this needs to be merged in order to make it work? @wvengen apache/cordova-plugin-inappbrowser#274

Bummer, too bad it didn't work out to get this merged.
Seems like it still may still be quite some effort to get the feature we need in Cordova, even on a custom branch.

Thing is, at the time postMessage was not reliable enough, so I used a custom protocol to signal from the inAppBrowser to the app. One workaround would be to navigate to a dummy page on scan, which the app will recognize and do the scanning. Gives a short empty screen.

I had hoped my Cordova changes would have been accepted by the time a new upgrade was due. But because my contribution was just before Cordova's WKWebView introduction, I didn't have the time to develop it there. Sad it didn't work out. Now we either need make these changes, and hope to get them upstream (a better chance, hopefully).

Or use a wholly different approach for the app, now this is becoming more time-consuming ... :( E.g. load within the app (not in an inAppBrowser - but that is a lot less safe with all these external scripts and embeds in the website), or use another website wrapping framework. It seems Cordova is fading indeed. Capacitor seems a similar alternative, and even mentions migration from Cordova. But it is still a lot of work.

Perhaps it makes sense to reconsider how much work we want to put in maintaining the app ...

@madhums

madhums commented May 3, 2023

Copy link
Copy Markdown
Member Author

Since the app is not at the risk of being unpublished any time soon, I'd say we can give capacitor a try in the coming time. ReactNative along side https://github.com/react-native-webview/react-native-webview is also an option to consider.

@wvengen

wvengen commented May 3, 2023

Copy link
Copy Markdown
Member

https://github.com/react-native-webview/react-native-webview

Ah, that's a good idea, now I understand you better :) It's a bit of overkill in terms of size (shipping all the RN code including its own JS runtime), but could be a good idea in terms of development speed - especially considering we have some RN experience.

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.

2 participants