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.

Sign up for GitHub

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

[Android 14] Update compileSdkVersion to 34 #740

Merged
merged 3 commits into from
Sep 12, 2023
Merged

[Android 14] Update compileSdkVersion to 34 #740

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4e8585f
Update compileSdkVersion to 34
Sep 1, 2023
5e63b71
Fix compilation errors for SDK 34
Sep 12, 2023
b388ab5
Enhance TextSizeSlider update by using deriveDimension
Sep 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions app/src/main/java/com/automattic/loop/photopicker/PhotoPickerActivity.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -405,8 +405,9 @@ private class FlingGestureListener extends SimpleOnGestureListener {
private static final int SWIPE_MIN_DISTANCE = 120;
private static final int SWIPE_THRESHOLD_VELOCITY = 200;

@Override public boolean onFling(MotionEvent e1, MotionEvent e2, float velocityX, float velocityY) {
if (e1 != null && e2 != null) {
@Override
public boolean onFling(@Nullable MotionEvent e1, @NonNull MotionEvent e2, float velocityX, float velocityY) {
if (e1 != null) {
if (e2.getY() - e1.getY() > SWIPE_MIN_DISTANCE
&& Math.abs(velocityY) > SWIPE_THRESHOLD_VELOCITY) {
// Top to bottom
Expand Down
2 changes: 1 addition & 1 deletion build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ subprojects {

ext {
minSdkVersion = 24
compileSdkVersion = 33
compileSdkVersion = 34
targetSdkVersion = 33
}

Expand Down
4 changes: 3 additions & 1 deletion stories/src/main/java/com/wordpress/stories/compose/ComposeLoopFrameActivity.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2292,7 +2292,9 @@ abstract class ComposeLoopFrameActivity : AppCompatActivity(), OnStoryFrameSelec
}

private inner class FlingGestureListener : GestureDetector.SimpleOnGestureListener() {
override fun onFling(e1: MotionEvent, e2: MotionEvent, velocityX: Float, velocityY: Float): Boolean {
override fun onFling(e1: MotionEvent?, e2: MotionEvent, velocityX: Float, velocityY: Float): Boolean {
if (e1 == null) return super.onFling(null, e2, velocityX, velocityY)
Copy link
Contributor

Choose a reason for hiding this comment

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

Question (❓): This still ends-up in a valid application flow, right? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Based on the logic here, we don't really know what to do if e1 is null, and the fallback is to just call the super anyway, so I kept calling the super implementation anyway.

I just realized though that when this fling is properly captured, the code is currently crashing because of a bug in a different part of the code (not related to this compileSdkVersion update). I will open a different PR to fix it.


if (e1.y - e2.y > SWIPE_MIN_DISTANCE && abs(velocityY) > SWIPE_THRESHOLD_VELOCITY) {
// Bottom to top
val ycoordStart = e1.y
Expand Down
2 changes: 1 addition & 1 deletion stories/src/main/java/com/wordpress/stories/compose/VideoRecordingControlView.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ class VideoRecordingControlView @JvmOverloads constructor(

override fun onSizeChanged(w: Int, h: Int, oldw: Int, oldh: Int) {
if (w != oldw || h != oldh) {
bitmap = Bitmap.createBitmap(w, h, Bitmap.Config.ARGB_8888)?.apply {
bitmap = Bitmap.createBitmap(w, h, Bitmap.Config.ARGB_8888).apply {
eraseColor(Color.TRANSPARENT)
viewCanvas = Canvas(this)
}
Expand Down
15 changes: 13 additions & 2 deletions stories/src/main/java/com/wordpress/stories/compose/text/TextSizeSlider.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.wordpress.stories.compose.text

import android.content.res.Resources
import android.os.Build
import android.util.TypedValue
import android.widget.SeekBar
import android.widget.SeekBar.OnSeekBarChangeListener
Expand Down Expand Up @@ -29,9 +30,19 @@ class TextSizeSlider(
})
}

@Suppress("DEPRECATION")
fun update() {
val fontSizeSp = (textView.textSize / resources.displayMetrics.scaledDensity).toInt()
seekBar.progress = (fontSizeSp - TEXT_SIZE_SLIDER_MIN_VALUE) / TEXT_SIZE_SLIDER_STEP
val fontSizeSp = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.UPSIDE_DOWN_CAKE) {
// this takes into account text specifics (such as font size multipliers in system settings)
TypedValue.deriveDimension(
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. Praise (❤️): Nice find and thanks for enhancing this update() function.
  2. Suggestion (💡): I would have recommended making this change in a separate commit and explaining the how & why in that commit's description. Maybe doing so would have made the extra comment included there obsolete, but to be honest, on this I am not 100% sure on that and whether I would have kept that comment around anyway... 🤷
  3. Question (❓): Was this change indeed fixing compilation errors, thus part of this commit, or was that change something totally different? As I understand, only adding the @Suppress("DEPRECATION") was needed to fix compilation errors, right? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would have recommended making this change in a separate commit and explaining the how & why in that commit's description.

That is a good point, I will rewrite the commit history to add this change in a separate commit. I will likely keep the comment though.

As I understand, only adding the @Suppress("DEPRECATION") was needed to fix compilation errors, right?

Well, adding the @Suppress("DEPRECATION") does indeed get rid of compilation errors, but doesn't really "fix" them in the sense of favoring the new API when possible, so I would consider this enhancement as part of the compileSdk changes. Wdyt? As you said in #2, I will likely move this enhancement to a separate commit, while keeping the @Suppress as part of the compileSdkcommit.

TypedValue.COMPLEX_UNIT_SP,
textView.textSize,
resources.displayMetrics
)
} else {
(textView.textSize / resources.displayMetrics.scaledDensity)
}
seekBar.progress = (fontSizeSp.toInt() - TEXT_SIZE_SLIDER_MIN_VALUE) / TEXT_SIZE_SLIDER_STEP
}

companion object {
Expand Down