-
Notifications
You must be signed in to change notification settings - Fork 531
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor: add clipboard helper and improve repo url copy #946
Conversation
app/src/main/java/com/lagradost/cloudstream3/ui/result/ResultFragmentPhone.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/lagradost/cloudstream3/ui/settings/SettingsFragment.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/lagradost/cloudstream3/ui/settings/SettingsUpdates.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/lagradost/cloudstream3/ui/settings/extensions/RepoAdapter.kt
Outdated
Show resolved
Hide resolved
val clip = ClipData.newPlainText(link.name, link.url) | ||
serviceClipboard.setPrimaryClip(clip) | ||
showToast(R.string.copy_link_toast, Toast.LENGTH_SHORT) | ||
val linkCopyLabel = UiText.DynamicString(link.name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
txt
?: return@setOnClickListener | ||
val clip = ClipData.newPlainText("logcat", text) | ||
serviceClipboard.setPrimaryClip(clip) | ||
val logcat = UiText.DynamicString("Logcat") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
txt
} | ||
} catch (t: Throwable) { | ||
Log.e("ClipboardService", "$t") | ||
showToast(R.string.clipboard_too_large) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not true, it can crash on multiple things like context!! or getSystemService!!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
better handling now
@@ -123,6 +129,20 @@ object UIHelper { | |||
) | |||
} | |||
|
|||
fun clipboardHelper(label: UiText, text: CharSequence) { | |||
try { | |||
val clip = ClipData.newPlainText(label.asString(context!!), text) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dont. do smth like
context?.let { ctx ->
}
or let ctx = context ?: return
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We never want to do !!
ctx.getSystemService<ClipboardManager>()?.setPrimaryClip(clip) | ||
|
||
if (Build.VERSION.SDK_INT <= Build.VERSION_CODES.S_V2) { | ||
showToast("$label $labelSuffix") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope, this is wrong, you cant use string like this on a UIText
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry for undersight, the asString(ctx) is necessary
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the version + commit addition, that's nice! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks fine, will merge after testing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works nice for me thanks, I don't know if I necessarily like the format of copying repos like Repo Name : https://codeberg.org/repo/raw/branch/builds/repo.json
, there is a bit extra whitespace and when it auto fills in when you go to paste the repo who have to manually remove the name now, but for most cases including the name is actually probably nice. Overall this does improve copying functionality.
improve repository copying
add clipboard util to ease copying
close #970