Repository navigation
Generate share links for CYOA tools (GT-3080) - #4588
tjohnson009 wants to merge 1 commit into
Conversation
Track the currently displayed page fragment via fragment lifecycle callbacks and combine it with the active manifest to build share links matching the knowgod.com URL pattern already supported by CyoaDeepLink: https://knowgod.com/{locale}/tool/v2/{tool}/{page} Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #4588 +/- ##
===========================================
+ Coverage 53.19% 53.25% +0.05%
===========================================
Files 440 440
Lines 11585 11607 +22
Branches 1960 1967 +7
===========================================
+ Hits 6163 6181 +18
Misses 4839 4839
- Partials 583 587 +4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| val tool = code ?: return null | ||
| val locale = locale ?: return null | ||
| return URI_SHARE_BASE.buildUpon() | ||
| .appendEncodedPath(locale.toString().lowercase(Locale.ENGLISH)) |
There was a problem hiding this comment.
locale.toString() might be wrong for locales with regions, if it's a java Locale object you will want to use locale.toLanguageTag(), if it's the fluidsonic Locale object, I'm not totally sure what format it uses for .toString()
| private val pageFragmentLiveData = MutableLiveData<CyoaPageFragment<*, *>?>(null) | ||
|
|
||
| private fun updatePageFragmentLiveData() { | ||
| val fragment = supportFragmentManager.primaryNavigationFragment as? CyoaPageFragment<*, *> | ||
| if (pageFragmentLiveData.value !== fragment) pageFragmentLiveData.value = fragment | ||
| } |
There was a problem hiding this comment.
you shouldn't be storing the fragment in a LiveData. I don't think your current implementation would leak anything, but storing an active fragment like this has potential to leak the fragment if it's not handled carefully.
You should probably just store what page the fragment is for
| .appendPath("tool") | ||
| .appendPath("v2") | ||
| .appendPath(tool) | ||
| .apply { page?.let { appendPath(it.id) } } |
There was a problem hiding this comment.
I don't think you are properly handling page collection pages here or card collection pages
|
This might need to be put on hold, I posted a message in the godtools channel about the web not supporting page collection pages, which means that those pages would be shared incorrectly. |
|
Going to wait in this until further notice |
2 participants
Summary
CYOA tools currently have no share link at all —
CyoaActivityinherits the emptyshareLinkUriLiveDatafromBaseToolActivity, so the share button never appears. This adds outgoing share link generation matching the knowgod.com URL pattern the incoming deep link parser (CyoaDeepLink) already supports:Changes
pageFragmentLiveData, updated viaFragmentLifecycleCallbacks(covers forward navigation, back navigation, and page dismissal)shareLinkUriLiveDatato combine the active manifest with the current page, so the link updates on both language changes and page navigationBaseToolActivitywiring once the link is non-nullThe
{position}path segment supported byCyoaDeepLinkfor card/page-collection pages is intentionally not included — the ticket's example URL omits it, and it can be added as a follow-up if needed.Tests
CyoaDeepLink.parseKnowGodDeepLink(same tool/page/locale)🎫 GT-3080
🤖 Generated with Claude Code