-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
History cache compose #11494
base: refactor
Are you sure you want to change the base?
History cache compose #11494
Conversation
add NP icon for Android Studio's NewUI
…-npe Fix crash in MediaSessionPlayerUi while destroying player
* Update Matrix room link, and prioritise it * Update Matrix room link in CONTRIBUTING.md * Prioritise Matrix in contribution doc too
Hotfix release v0.27.2
Likely related to mozilla/rhino@01a7b20 but I am not completely sure. I tested the app and it works well, so I think that org.mozilla.javascript.JavaToJSONConverters is not used really. This is the full list of errors: Missing class java.beans.BeanDescriptor (referenced from: java.lang.Object org.mozilla.javascript.JavaToJSONConverters.lambda$static$4(java.lang.Object)) Missing class java.beans.BeanInfo (referenced from: java.lang.Object org.mozilla.javascript.JavaToJSONConverters.lambda$static$4(java.lang.Object)) Missing class java.beans.IntrospectionException (referenced from: java.lang.Object org.mozilla.javascript.JavaToJSONConverters.lambda$static$4(java.lang.Object)) Missing class java.beans.Introspector (referenced from: java.lang.Object org.mozilla.javascript.JavaToJSONConverters.lambda$static$4(java.lang.Object)) Missing class java.beans.PropertyDescriptor (referenced from: java.lang.Object org.mozilla.javascript.JavaToJSONConverters.lambda$static$4(java.lang.Object))
returnActivity was removed in 463dd8e
…y-test Remove outdated returnActivity test code
…in multiple README.md's translated into different languages (TeamNewPipe#11487) Link to contribution notes wasn't working * Update README.de.md, fix grammar in README.de.md * Update README.asm.md * Update README.fr.md * Update README.hi.md * Update README.it.md * Update README.pa.md * Update README.pt_BR.md * Update README.ru.md * Update README.sr.md --------- Co-authored-by: Tobi <[email protected]>
@snaik20 can you review this PR? |
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 looks pretty decent, thank you! In particular, the addition of Hilt will be useful.
That being said, I've suggested some improvements.
class OpenErrorActivity( | ||
private val context: Context, | ||
) { | ||
operator fun invoke(errorInfo: ErrorInfo) { | ||
val intent = Intent(context, ErrorActivity::class.java) | ||
intent.putExtra(ErrorActivity.ERROR_INFO, errorInfo) | ||
intent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK) | ||
|
||
context.startActivity(intent) | ||
} | ||
} |
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 would be better as a static method in the error activity class itself.
val clickModifier = if (enabled) { | ||
Modifier.clickable { onClick() } | ||
} else { | ||
Modifier | ||
} | ||
Row( | ||
modifier = clickModifier.then(modifier), |
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.
val clickModifier = if (enabled) { | |
Modifier.clickable { onClick() } | |
} else { | |
Modifier | |
} | |
Row( | |
modifier = clickModifier.then(modifier), | |
Row( | |
modifier = Modifier | |
.clickable(enabled, onClick = onClick) | |
.then(modifier), |
) = ComposeView(requireContext()).apply { | ||
setViewCompositionStrategy(ViewCompositionStrategy.DisposeOnViewTreeLifecycleDestroyed) | ||
setContent { | ||
AppTheme { | ||
HistoryCacheSettingsScreen( | ||
modifier = Modifier.fillMaxSize() | ||
) | ||
} | ||
} | ||
} |
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.
) = ComposeView(requireContext()).apply { | |
setViewCompositionStrategy(ViewCompositionStrategy.DisposeOnViewTreeLifecycleDestroyed) | |
setContent { | |
AppTheme { | |
HistoryCacheSettingsScreen( | |
modifier = Modifier.fillMaxSize() | |
) | |
} | |
} | |
} | |
) = content { | |
AppTheme { | |
HistoryCacheSettingsScreen(modifier = Modifier.fillMaxSize()) | |
} | |
} |
This requires the Fragment Compose dependency.
package org.schabi.newpipe.settings.domain.usecases.get_preference | ||
|
||
import kotlinx.coroutines.flow.Flow | ||
|
||
fun interface GetPreference<T> { | ||
operator fun invoke(key: Int, defaultValue: T): Flow<T> | ||
} |
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.
Using DataStore might be a better option instead of adding custom classes for handling preferences.
What is it?
Description of the changes in your PR
Before/After Screenshots/Screen Record
Before:
After:
Fixes the following issue(s)
APK testing
The APK can be found by going to the "Checks" tab below the title. On the left pane, click on "CI", scroll down to "artifacts" and click "app" to download the zip file which contains the debug APK of this PR. You can find more info and a video demonstration on this wiki page.
Due diligence