Skip to content

Conversation

@juuce79
Copy link
Contributor

@juuce79 juuce79 commented Mar 14, 2025

Conversion of BarcodeSelectorActivity from java to kotlin code.

  • Manually tested in Android Studio using emulator with APIs 24, 30, 35 and 36
  • Manually tested on physical device using API 33
  • All tests in BarcodeSelectorActivityTests pass
  • Code has been kept as close to the original Java code as possible (structurally)
    barcode_selector_activity.pdf

Have tried to keep the code as close to the original as possible while using kotlin code instead of java code.
- added new line at EOF.
- changed if-else statement to `when` block for cleaner and more easily extensible code (lines 57-63).
- now using `isVisible` extension method from the core-ktx library to make the logic more clean and simple (lines 48 & 50).
- now using correct view binding pattern for activities (line 22).
- removed `_binding = null` from `onDestroy()` function (line 71).
…tonVisibilityBasedOnBuildConfig` to use `isVisible` instead of checking visibility property directly with `View.VISIBLE/GONE` comparisons. This makes tests more consistent with implementation code in `AboutActivity.kt` and improves test readability.
…tlin_conversion/barcode_selector_activity_kt_conversion
@TheLastProject
Copy link
Member

You seem to have a conflict with BarcodeSelectorActivity.java, which is weird, because that file hasn't changed in 3 years: https://github.com/CatimaLoyalty/Android/commits/main/app/src/main/java/protect/card_locker/BarcodeSelectorAdapter.java

Your commit history seems quite messy, there are commits in there that have already been merged. Can you rebase it on top of the main branch?

@TheLastProject
Copy link
Member

Hey @juuce79, haven't heard from you for a while. Are you still working on this? Just want to know if I should add your WIP to #2483 :)

@TheLastProject TheLastProject marked this pull request as draft July 26, 2025 09:43
@TheLastProject
Copy link
Member

Partially superseded by #2585 (the tests weren't converted in that MR, but they are here)

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.

2 participants