quran / quran-ios

QuranEngine is the engine powering the Quran.com iOS app.
Apache License 2.0
462 stars 160 forks source link

Custom URL Scheme Implementation #657

Closed hassan31 closed 2 months ago

hassan31 commented 2 months ago

Implemented custom url scheme to support launching the Quran Application from outside the application For example, enter this in browser quran-ios:// and it will redirect you to the Quran Application

Example Video

https://github.com/user-attachments/assets/cafcc9f4-5c24-4ebd-b6fb-1b680ab69f7c

hassan31 commented 2 months ago

@mohamede1945 could you please review this pr and approve so that it can be merged. Thanks

mohamede1945 commented 2 months ago

Salam alikom Muhammad, thank you for the contribution. But the code is empty. There's nothing implemented to parse the query parameters and navigate to a specific Ayah as mentioned in #348.

hassan31 commented 2 months ago

@mohamede1945 Wa Alaikumuslaam, Yes, this is the partial implementation, in which I have launched the application from outside link. In followup pr I will write the code to parse. So if you could approve and merge that would be really great.

mohamede1945 commented 2 months ago

Oh actually, please move the validation, parsing and navigation code to the QuranEngine instead of the example app. The example app would simply just implement the delegate method but then calls the QuranEngine to do the actual logic, this way the code can be shared among different apps.

hassan31 commented 2 months ago

Oh actually, please move the validation, parsing and navigation code to the QuranEngine instead of the example app. The example app would simply just implement the delegate method but then calls the QuranEngine to do the actual logic, this way the code can be shared among different apps.

AOA @mohamede1945 I have addressed your comments, kindly review again. Also regarding move the validation, parsing and navigation code to QuranEngine, i can see the same code there, but in pr it reflects in Example/QuranEngine app. May be if you guide how to move then I would do. Please check below screenshot.

Screenshot 2024-09-08 at 12 16 07 AM
mohamede1945 commented 2 months ago

You see here how the code in the example project delegates the business logic to the DownloadManager and ReadingResourcesService. These live in the QuranEngine library not in the Example project. The example project just delegates the call to these services.

In url scheme navigation logic, I think it makes sense to live in the AppStructureFeature.

hassan31 commented 2 months ago

You see here how the code in the example project delegates the business logic to the DownloadManager and ReadingResourcesService. These live in the QuranEngine library not in the Example project. The example project just delegates the call to these services.

In url scheme navigation logic, I think it makes sense to live in the AppStructureFeature.

@mohamede1945 can we connect through some communication tools, like slack or google meet, I wanted to learn more about the structure?

hassan31 commented 2 months ago

@mohamede1945 are you still available to review the latest changes?

mohamede1945 commented 2 months ago

Jazak Allah khyrn for the changes. I'm going to merge it after CI finishes. You can reach out to me on discord with the same username used here in github.

codecov[bot] commented 2 months ago

Codecov Report

Attention: Patch coverage is 0% with 12 lines in your changes missing coverage. Please review.

Project coverage is 39.69%. Comparing base (d9fc366) to head (5223a5b). Report is 17 commits behind head on main.

Files with missing lines Patch % Lines
...res/AppStructureFeature/Launch/LaunchBuilder.swift 0.00% 12 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #657 +/- ## ========================================== - Coverage 40.92% 39.69% -1.24% ========================================== Files 525 526 +1 Lines 20880 19453 -1427 ========================================== - Hits 8546 7721 -825 + Misses 12334 11732 -602 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

hassan31 commented 2 months ago

Jazak Allah khyrn for the changes. I'm going to merge it after CI finishes. You can reach out to me on discord with the same username used here in github.

Merge is blocked, because of some checks are not successful. I also installed the swiftlint, but looks good on my side. May be you can help to find what is wrong with which format.

Also, I sent you request on discord. Please accept. Jazakallah Khair.