Skip to content
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

full sdk 34 support #4224

Merged
merged 22 commits into from
Feb 23, 2024
Merged

full sdk 34 support #4224

merged 22 commits into from
Feb 23, 2024

Conversation

connyduck
Copy link
Collaborator

@connyduck connyduck commented Jan 9, 2024

builds upon work from #4082

Additionally fixes some deprecations and adds support for predictive back. I also refactored how the activity transitions work because they are closely related to predictive back. The awkward finishWithoutSlideOutAnimation is gone, activities that have been started with slide in will now automatically close with slide out.

To test predictive back you need an emulator or device with Sdk 34 (Android 14) and then enable it in the developer settings.

Predictive back requires the back action to be determined before it actually occurs so the system can play the right predictive animation, which made a few reorganisations necessary.

closes #4082
closes #4005
unlocks a bunch of dependency upgrades that require sdk 34

@connyduck connyduck marked this pull request as ready for review February 8, 2024 12:19
Copy link
Collaborator

@charlag charlag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not test it on new SDK but on my Android 13 seems to work just fine.

I still think we need to change our transitions, vanilla masto does it well, our is only slide without alpha, takes 300ms and uses weird interpolation.

@@ -167,8 +174,14 @@ private static int textStyle(String name) {
}

public void startActivityWithSlideInAnimation(@NonNull Intent intent) {
// the new transition api needs to be called by the activity the is the result of the transition
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some typo here

@@ -73,6 +75,11 @@ public abstract class BaseActivity extends AppCompatActivity implements Injectab
protected void onCreate(@Nullable Bundle savedInstanceState) {
super.onCreate(savedInstanceState);

if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.UPSIDE_DOWN_CAKE && getIntent().getBooleanExtra(OPEN_WITH_SLIDE_IN, false)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd suggest to make helpers like supportsTransitionApi() but no stress

@@ -690,6 +711,14 @@ class ComposeActivity :
)
}

private fun updateOnBackPressedCallbackState(confirmation: ConfirmationKind) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need to pass confirmation in? To me it was confusing why bottom sheets should re-toggle the state that we already observe from viewModel.

}

fun toggleMarkSensitive() {
this.markMediaAsSensitive.value = this.markMediaAsSensitive.value != true
}

fun handleCloseButton(contentText: String?, contentWarning: String?): ConfirmationKind {
return if (didChange(contentText, contentWarning)) {
fun contentUpdated(newContent: String?) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

new methods don't sound neither like others in imperative (updateContent) nor like callbacks (onContentUpdated). I'd go with imperative

R.anim.slide_from_right,
R.anim.slide_to_left
)
(activity as? BaseActivity)?.startActivityWithSlideInAnimation(intent)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we could probably put it as an extension on Activity so that we don't have to cast it all the time

Copy link
Collaborator

@Tak Tak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems ok here as well? (but I only have android 11)

@connyduck connyduck requested a review from charlag February 20, 2024 10:34
@connyduck connyduck merged commit b976fe5 into develop Feb 23, 2024
3 checks passed
@connyduck connyduck deleted the sdk34 branch February 23, 2024 09:27
connyduck added a commit that referenced this pull request Mar 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrade targetSdk to 34 (Android 14) and support predictive back
4 participants