App + Library conversion to Kotlin

[anderbytes]

407 continued

Did the Kotlin conversion on the app only and did some manual testing. No issues found so far.

[anderbytes]

@lucianocheng @burhanrashid52 please check this and do your testing.

[anderbytes]

Left some initial comments.

Ran some tests, the images aren't saving for me on the Android Version 30 emulator. Freezes at "Saving" with the spinner.

@lucianocheng can you post the logcat of the error, so we can investigate with more details?

[lucianocheng]

Left some initial comments.

Ran some tests, the images aren't saving for me on the Android Version 30 emulator. Freezes at "Saving" with the spinner.

@lucianocheng can you post the logcat of the error, so we can investigate with more details?

So it's interesting, there wasn't a logcat error it just froze up. Tried it twice with Android 30 on the emulator.

I think it's a wait lock or a for loop or something. Could you see if you can replicate? Does exporting work for you with one sticker added?

[anderbytes]

Left some initial comments. Ran some tests, the images aren't saving for me on the Android Version 30 emulator. Freezes at "Saving" with the spinner.

@lucianocheng can you post the logcat of the error, so we can investigate with more details?

So it's interesting, there wasn't a logcat error it just froze up. Tried it twice with Android 30 on the emulator.

I think it's a wait lock or a for loop or something. Could you see if you can replicate? Does exporting work for you with one sticker added?

Maybe it didn't "FREEZE" as an error, it just never reached the hideLoading function, that gets the Dialog out of the way.

Can you tell me the exact details of the emulator so I can replicate here?

Also... In your place I'd distribute several Log.d inside FileSaveHelper, and try to understand exactly what it is doing and what it is not, to get some good idea on what is the problematic line.

[lucianocheng]

Can you tell me the exact details of the emulator so I can replicate here?

Pixel 4, API 30, Android 11

Also... In your place I'd distribute several Log.d inside FileSaveHelper, and try to understand exactly what it is doing and what it is not, to get some good idea on what is the problematic line.

If you can replicate it, I would recommend you put some stop points in FileSaveHelper and look at it with the debugger. If you can't let me know and I'll try again.

[anderbytes]

Can you tell me the exact details of the emulator so I can replicate here?

Pixel 4, API 30, Android 11

Also... In your place I'd distribute several Log.d inside FileSaveHelper, and try to understand exactly what it is doing and what it is not, to get some good idea on what is the problematic line.

If you can replicate it, I would recommend you put some stop points in FileSaveHelper and look at it with the debugger. If you can't let me know and I'll try again.

I've debugged it and the situation is: In the new code,

Can you tell me the exact details of the emulator so I can replicate here?

Pixel 4, API 30, Android 11

Also... In your place I'd distribute several Log.d inside FileSaveHelper, and try to understand exactly what it is doing and what it is not, to get some good idea on what is the problematic line.

If you can replicate it, I would recommend you put some stop points in FileSaveHelper and look at it with the debugger. If you can't let me know and I'll try again.

I've did my homework and the situation is: with the new Kotlin code, writing files locally requires that the ActivityCompat.checkSelfPermission(applicationContext, Manifest.permission.WRITE_EXTERNAL_STORAGE) is GRANTED (around 12th line of saveImage() method in EditImageActivity.kt)

It even put a TODO there, exactly because when the permission is NOT granted, it needs to be asked to the user, or a specific exception has to be raised (to handle informing the user that the action was not possible)

I'll try to implement this permission-asking or create a Toast telling him that it lacks permissions.

[lucianocheng]

I'll try to implement this permission-asking or create a Toast telling him that it lacks permissions.

👍

Also the code tests on Github Actions aren't being executed (see the Kotlin conversion PR for an example https://github.com/burhanrashid52/PhotoEditor/pull/417).

@burhanrashid52 do we need to do something special to trigger the tests on the PR? Or should they run automatically?

[anderbytes]

I'll try to implement this permission-asking or create a Toast telling him that it lacks permissions.

👍

Also the code tests on Github Actions aren't being executed (see the Kotlin conversion PR for an example #417).

@burhanrashid52 do we need to do something special to trigger the tests on the PR? Or should they run automatically?

I'm illiterate in "Github Actions", don't have a clue about them

[anderbytes]

There, @lucianocheng , I adjusted it It happens that the old code already took care of only asking explicit disk permissions on API Level lesser than 28, but the IDE didn't detect it and at the moment of the Kotlin conversion I accepted the suggestion of the IDE to ask for the permission, which now I see is unecessary.

So I removed the useless extra-check and now everything works here in Saving, be it in API Level < 28 ou >= 28

ps: the endless Saving was no error or crash, just the lack of a hideLoading() as I suspected before

[burhanrashid52]

@lucianocheng Thanks for pointing it out. I've fixed it in github yml. @anderbytes You just need to push a new commit to trigger the GitHub workflow.

[anderbytes]

@lucianocheng Thanks for pointing it out. I've fixed it in github yml. @anderbytes You just need to push a new commit to trigger the GitHub workflow.

I'll do now some little danger-free cleaning on the code, to activate the commit you need, ok?

[anderbytes]

Actually, I'll change every

@lucianocheng Thanks for pointing it out. I've fixed it in github yml. @anderbytes You just need to push a new commit to trigger the GitHub workflow.

I'll do now some little danger-free cleaning on the code, to activate the commit you need, ok?

Actually, I'll change every != null to a better-written .let of Kotlin this will be today's cleaning

[burhanrashid52]

@anderbytes I think pushing commit is not triggering the GitHub action workflow:(. You need to recreate a new PR.

[lucianocheng]

@anderbytes I think pushing commit is not triggering the GitHub action workflow:(. You need to recreate a new PR.

@burhanrashid52 wow I'm kind of shocked that Github expects you to create a new PR for running updated actions configuration. Is there another option? Can @anderbytes pull from master to pull in the yml changes?

[burhanrashid52]

@anderbytes I think pushing commit is not triggering the GitHub action workflow:(. You need to recreate a new PR.

@burhanrashid52 wow I'm kind of shocked that Github expects you to create a new PR for running updated actions configuration. Is there another option? Can @anderbytes pull from master to pull in the yml changes?

Actually, it's working correctly. I added a PR trigger after the PR was created hence it is not able to catch the event.

[anderbytes]

I can't create a new pull request for the same commits/branch because Github detects there's this one active with the same changes.

I'll find some time today morning to stash all my changes locally, remove my branch then recreate it... maybe then I'll be able to ask for a new pull.