MBL-2851: Re-enable support for Discover deep links #2456
Conversation
tonyteate
force-pushed
the
tony/mbl-2851
branch
from
a3ce765 to
d067038
Compare
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2456 +/- ##
=========================================
Coverage 64.27% 64.27%
- Complexity 2403 2405 +2
=========================================
Files 378 378
Lines 28744 28754 +10
Branches 4145 4147 +2
=========================================
+ Hits 18474 18483 +9
Misses 8011 8011
- Partials 2259 2260 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
ycheng-kickstarter
left a comment
ycheng-kickstarter
left a comment
There was a problem hiding this comment.
Looked great, just make sure to actually ticket the todos lest they be forgotten forever
| fun testUri_isDiscoverUri() { | ||
| val discoverBaseUri1 = Uri.parse("https://www.ksr.com/discover") | ||
| val discoverBaseUri2 = Uri.parse("https://www.ksr.com/discover/") | ||
| val discoverScopeUri = Uri.parse("https://www.ksr.com/discover/ending-soon") |
There was a problem hiding this comment.
question about the url of these tests 🤔 , in which scenario do we see something like https://www.ksr.com/discover were the domain is www.ksr.com?
ksr is used as schema instead of https but never saw it as domain
There was a problem hiding this comment.
Great question. For flexibility, isDiscoverUri() follows a pattern in similar parts of UriExt by calling isKickstarterUri(webEndpoint) in its implementation. So in the test case we also follow the pattern of using in the top-level webEndpoint variable defined in the test suite (L72), which is set to "https://www.ksr.com".
| binding.root.viewTreeObserver.addOnGlobalLayoutListener(object : ViewTreeObserver.OnGlobalLayoutListener { | ||
| override fun onGlobalLayout() { | ||
| binding.root.viewTreeObserver.removeOnGlobalLayoutListener(this) | ||
| viewModel.provideIntent(intent) |
📲 What
Re-enable support for Discover deep links
🤔 Why
The initial introduction of a Splash Screen in #2443 removed support for Discover deep links, which were originally supported by
DiscoveryActivityvia Intent Filter, but only for theksr://scheme. Here we re-enable support for Discover deep links (and app links), through the splash screen, for both theksr://andhttps://schemes.🛠 How
Update the Intent-processing logic in
SplashScreenViewModelto filter for Discover URLs via regex and emit anOptional<Uri>. When present, a new instance ofDiscoveryActivityis launched withaction.VIEWand the Uri as Data; when empty, delegate to the existing logic for starting a newDiscoveryActivity(withaction.MAIN).DiscoveryViewModelalready knows how to handle these Intents, including parsing and transforming Uris intoDiscoveryParams. To help mitigate the existing possibility of race condition between handlingDiscoveryParamsand rendering theViewPagerwith the correct tab selected, we updateDiscoveryActivityto callprovideIntent()only after a first-pass render of the view tree has completed.👀 See
Before 🐛
Screen_recording_20251208_124014.mp4
After 🦋 |
Screen_recording_20251208_124319.mp4
📋 QA
Setup
'Explore the app'
*.kickstarter.comandwww.kickstarter.comare selected.
Discover Category via App Link
https://www.kickstarter.com › discover › categories › journalismDiscover Category via Banner Deep Link
https://www.kickstarter.comksr://scheme)Discover Sort via ADB
Open the app (will also work if the app is terminated)
Run the following command to simulate opening an LCM deep link to a sorted Discover
Results:
Story 📖
[MBL-2851] [Fast Follow] Support LCM deep links to Discover - Jira