fix internal image zoom by ilyaZar · Pull Request #3184 · CatimaLoyalty/Android

ilyaZar · 2026-07-01T09:55:26Z

per the discussion; happy to change the PR with changes related to other issues (though I only digged up the 2156 one)

thanks for the great app, I really like it !

TheLastProject · 2026-07-01T10:06:47Z

Any chance the new file could be in Kotlin? Java is being deprecated by Google so I'd rather not accept more new Java code where it can be easily prevented.

I haven't had time to review the actual change yet, but it seems this adds even more complexity to the view activity? Given Google is reducing support for Android XML and the view activity still depends on Android XML (and still needs to be migrated, see #2483), I will need to look closely if this won't make migrating the view activity harder. I'll get back later after a basic review, this might need its own view to prevent the already overly complex view activity to become even harder to migrate.

ilyaZar · 2026-07-01T10:19:25Z

thanks for coming back so quickly

yes will do, but I'll need the next weekend for it . maybe its wise for me to rework this first, before you spend time looking at it but if youhave the time now that would be great of course too

I will think about the issues and implications of the view activity and the #2483 thing as well; thanks for steering me toward this one

update: just changed to a draft

TheLastProject · 2026-07-04T15:28:04Z

Had some time to test it. The zooming looks good, but I do worry about the extra complexity. Maybe the zooming can have its own UI, just a very minimal Jetpack Compose UI screen (look at AboutActivity for that) and we can just launch that from the view screen? I think that may be better for maintainability.

ilyaZar · 2026-07-04T18:48:49Z

Hi , thanks for your comments again @TheLastProject

let me know what else you'd like to have in there, whenever you have time

TheLastProject

I'll have to take a deeper look soon, but the general flow seems reasonable. Noticing one consistency-with-rest-of-codebase issue. I'll do a test and deeper review later :)

TheLastProject · 2026-07-05T15:21:09Z

+@Composable
+fun LoyaltyCardImageScreen(
+    bitmap: Bitmap,
+    contentDescription: String,
+    onBack: () -> Unit,
+) {


Please put the ui elements in the compose directory as done with other activities.

yea true, thx, see commit below

- keep new compose ui with existing compose code

TheLastProject

This is great, it works really well and the code is easy to understand for me. I'm quite happy to see more functionality move out of the LoyaltyCardView.java activity, so that's great too.

I found a few small issues and I think some file renames might be good for future maintainability but otherwise this is basically ready to be merged I think :)

TheLastProject · 2026-07-08T17:29:07Z

+
+        currentWindow.setSoftInputMode(WindowManager.LayoutParams.SOFT_INPUT_STATE_ALWAYS_HIDDEN)


We're not doing any input so this is unneeded

Suggested change

currentWindow.setSoftInputMode(WindowManager.LayoutParams.SOFT_INPUT_STATE_ALWAYS_HIDDEN)

TheLastProject · 2026-07-08T17:36:25Z

        switch (imageType) {
            case NONE:
+            case BARCODE:
                return;
            case ICON:
-                file = Utils.retrieveCardImageAsFile(this, loyaltyCardId, ImageLocationType.icon);
-                break;
+                startImageActivity(ImageLocationType.icon);
+                return;
            case IMAGE_FRONT:
-                file = Utils.retrieveCardImageAsFile(this, loyaltyCardId, ImageLocationType.front);
-                break;
+                startImageActivity(ImageLocationType.front);
+                return;
            case IMAGE_BACK:
-                file = Utils.retrieveCardImageAsFile(this, loyaltyCardId, ImageLocationType.back);
-                break;
-            case BARCODE:
-                Toast.makeText(this, R.string.barcodeLongPressMessage, Toast.LENGTH_SHORT).show();
+                startImageActivity(ImageLocationType.back);
                return;
        }


Android Studio complains about the unnecessary return. How about this? Then we even have logging in case something stupid gets done in the future :)

private void openImage(LoyaltyCardImageType imageType) { switch (imageType) { case ICON: startImageActivity(ImageLocationType.icon); break; case IMAGE_FRONT: startImageActivity(ImageLocationType.front); break; case IMAGE_BACK: startImageActivity(ImageLocationType.back); break; default: Log.w(TAG, "openImage called with unsupported image type"); } }

TheLastProject · 2026-07-08T17:37:37Z

+    @StringRes contentDescriptionRes: Int,
+    onBack: () -> Unit,
+) {
+    var scale by remember { mutableStateOf(1F) }


Android Studio suggestion:

Prefer mutableFloatStateOf instead of mutableStateOf

TheLastProject · 2026-07-08T17:40:38Z

+            Icon(
+                imageVector = Icons.AutoMirrored.Filled.ArrowBack,
+                contentDescription = stringResource(R.string.back)
+            )


The back icon remains black in dark mode. Maybe using the CatimaTopAppBar like in the AboutActivity makes sense? Also standardizes the UI more, which should help future maintenance.

TheLastProject · 2026-07-08T17:44:20Z

Maybe move this into it's own cardimageview package and rename it to LoyaltyCardImageViewActivity.kt, given how stand-alone it is and to make clear this is specifically for viewing images, nothing else?

TheLastProject · 2026-07-08T17:47:28Z

Sorry, I asked you already to move this to compose, but I think it may be best for this to use the same package as the activity. So maybe move this to compose/cardimageview/LoyaltyCardImageViewActivity.kt?

TheLastProject · 2026-07-08T17:47:45Z

+import protect.card_locker.R
+
+@Composable
+fun LoyaltyCardImageScreen(


Given with the other renames, maybe LoyaltyCardImageViewScreen?

ilyaZar · 2026-07-09T10:15:49Z

thanks ! i will try to get this done by the next weekend

ilyaZar marked this pull request as draft July 1, 2026 19:05

ilyaZar added 3 commits July 4, 2026 20:43

feat keep card image zoom in catima

6cbbeb1

test prove image taps open the viewer

89c97c4

refactor drop old gallery strings

27bcb6c

ilyaZar force-pushed the fix/internal-image-zoom branch from a6c89ce to 27bcb6c Compare July 4, 2026 18:47

ilyaZar marked this pull request as ready for review July 4, 2026 18:48

TheLastProject reviewed Jul 5, 2026

View reviewed changes

refactor: align image viewer ui package

a8368b6

- keep new compose ui with existing compose code

ilyaZar mentioned this pull request Jul 6, 2026

refactor for consistency move about compose ui #3196

Closed

TheLastProject reviewed Jul 8, 2026

View reviewed changes


		currentWindow.setSoftInputMode(WindowManager.LayoutParams.SOFT_INPUT_STATE_ALWAYS_HIDDEN)

Uh oh!

Uh oh!

Conversation

ilyaZar commented Jul 1, 2026

Uh oh!

TheLastProject commented Jul 1, 2026

Uh oh!

ilyaZar commented Jul 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TheLastProject commented Jul 4, 2026

Uh oh!

ilyaZar commented Jul 4, 2026

Uh oh!

TheLastProject left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TheLastProject left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ilyaZar commented Jul 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ilyaZar commented Jul 1, 2026 •

edited

Loading