-
Notifications
You must be signed in to change notification settings - Fork 118
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
Update compile sdk to 33 #1037
Update compile sdk to 33 #1037
Conversation
"primaryClip" was converted to val after API 28.
Uri's path has been converted to null after API 28.
Handler's removeCallbacks function don't accept nullable runnable after API 28.
Some functions has converted to nullable after API 28.
Some MetricAffectingSpan functions have changed after API 28.
Some LineBackgroundSpan functions have changed after API 28.
Creating Handler with empty constructor was deprecated. It was resolved to adding Looper.getMainLooper() as looper to the construction.
This deprecated warning is suppressed, that is, instead of being resolved, since a resolution would require a proper migration.
Activity's onBackPressed was deprecated on Android 13.
👋🏻 @khaykov!
I want to test its effect to be sure it still works as expected, but I couldn't find what this line does exactly. It should resize the block editor position on the screen when the keyboard is shown up, so I removed this line and also removed
Do you know how I can test what this line does? |
Hey @irfano ! I remember that one :) I was testing updating |
LayoutParams.SOFT_INPUT_ADJUST_RESIZE was deprecated on API 30. Using "View.OnApplyWindowInsetsListener" is the recommended way to update it but it requires activity.window that we can't access from AztecsText. It suppressed for now until the migration will be mandatory.
LayoutParams.SOFT_INPUT_ADJUST_RESIZE was deprecated in API level 30. A theme added to the dialog to keep the same behavior.
Thank you @khaykov! I found out its function and explained it in the "Test" section. |
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.
👋 @irfano !
I have reviewed and tested this PR as per the instructions, everything works as expected, good job! 🌟 🌟 🌟
I have left just one suggestion (💡) for you to consider with lots of praise (❤️)! I am going to approve this PR anyway, since none is blocking. I am NOT going to merge this PR yet to give you some time to apply any of my suggestions. However, feel free to ignore them and merge the PR yourself.
// gesture feature. | ||
isEnabled = false | ||
onBackPressedDispatcher.onBackPressed() | ||
isEnabled = true |
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.
- Praise (❤️): You are becoming a pro at that, aren't you! 😃
- Suggestion (💡): Maybe creating a
CompatExtensions.kt
utility class on Aztec, with aonBackPressedCompat(...)
extension function, copy-pasting it from here, is the right move for this repo too, just to keep this consistent across every repo, wdyt? 🤔 PS: I know this might seem an overkill as it only applies to this specific and single instance of suchonBackPressedDispatcher
functionality within this repo, so if you want to skip it instead, I would totally understand.
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.
- Praise (❤️): You are becoming a pro at that, aren't you! 😃
and you're becoming a pro at reviewing that. 😁
- Suggestion (💡): Maybe creating a CompatExtensions.kt utility class on Aztec, with a onBackPressedCompat(...) extension function, copy-pasting it from here, is the right move for this repo too, just to keep this consistent across every repo, wdyt?
Good advice, but this code is already in the demo app, which consists of only one package and four files. Therefore, I will choose to avoid adding a new file.
@@ -2099,7 +2098,7 @@ open class AztecText : AppCompatEditText, TextWatcher, UnknownHtmlSpan.OnUnknown | |||
|
|||
@SuppressLint("InflateParams") | |||
fun showBlockEditorDialog(unknownHtmlSpan: UnknownHtmlSpan, html: String = "") { | |||
val builder = AlertDialog.Builder(context) | |||
val builder = AlertDialog.Builder(context, R.style.ResizableDialogTheme) |
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.
Praise (❤️): Thanks for taking the time to resolve this suppression with the most appropriate fix for this specific case, you rock! 🥇
Fix
This updates
compileSdk
to 33 and makes the required changes.getExternalStoragePublicDirectory()
warning was suppressed and opened an issue. It's a low-priority task since it's on the sample activity.LayoutParams.SOFT_INPUT_ADJUST_RESIZE
was deprecated in API level 30. I suppressed it at first but then found a way to resolve it.Test
Being able to build and basic smoke test should be enough.
Updating
LayoutParams.SOFT_INPUT_ADJUST_RESIZE
may be worth a special test. To test:Review
@ParaskP7
Make sure strings will be translated:
strings.xml
as a part of the integration PR.