Add copy barcode on long press barcode & description #2445
Conversation
LucasVermersch
commented
Apr 15, 2025
LucasVermersch
commented
- Feature : Copy the barcode value when long pressing the barcode image
- Feature : Copy the barcode value when long pressing the barcode description
- Test : Add test for the previous features
|
As I stated in the issue, long-pressing the main image already had existing functionality. We should try to minimize breaking existing functionality, so please do not change that behaviour. As written in the issue, please
I'm also not sure if we should show a toast on modern Android versions, as Android already shows its own UI element to show it copied the content (though not sure which version added that), but I guess it's fine to figure that out later and just always show it for now. |
There was a problem hiding this comment.
Thanks for the contribution! Mostly looks good, just some small notes.
Also, I noticed another bug during review, so we may as well clean that up (but GitHub sadly doesn't let me comment on unchanged lines):
In this part:
binding.mainImageDescription.setOnClickListener(v -> {
if (mainImageIndex != 0) {
// Don't show cardId dialog, we're displaying something else
return;
}
we should probably just remove the if statement:
if (mainImageIndex != 0) {
// Don't show cardId dialog, we're displaying something else
return;
}
| @@ -146,6 +147,21 @@ public void onMainImageTap() { | |||
| openImageInGallery(imageType); | |||
| } | |||
|
|
|||
| private boolean copyBarcodeToClipBoard(){ | |||
| if (imageTypes.get(mainImageIndex) == ImageType.BARCODE) { | |||
There was a problem hiding this comment.
This seems to crash if the card has "Barcode Type" set to "No barcode" and has no photo too.
I was about to suggest using the mainImageIndex != 0 check from binding.mainImageDescription.setOnClickListener but I just realized that that check is wrong too (it fails if there is no barcode but an image).
Maybe, given that field does not currently do anything, and copying the text "Front image" or "Back image" is useless... maybe just remove this check altogether and always copy the barcode on long-press?
| @@ -350,12 +366,16 @@ public void onStopTrackingTouch(SeekBar seekBar) { | |||
| }); | |||
|
|
|||
| binding.mainImage.setOnClickListener(view -> onMainImageTap()); | |||
|
|
|||
There was a problem hiding this comment.
Maybe I'm weird, but I quite like the setOnClickListener and setOnLongClickListener of the same object being directly below each other :)
| // This long-press was originally only intended for when Talkback was used but sadly limiting | ||
| // this doesn't seem to work well | ||
| binding.mainImage.setOnLongClickListener(view -> { | ||
| setMainImage(true, true); | ||
| return true; | ||
| }); | ||
|
|
||
| binding.mainImageDescription.setOnLongClickListener(view -> copyBarcodeToClipBoard()); |
There was a problem hiding this comment.
I think this would be better to move directly below binding.mainImageDescription.setOnClickListener so everything of binding.mainImageDescription is in one place.
| @@ -1389,4 +1391,51 @@ public void importCardOldFormat() { | |||
| checkAllFields(activity, ViewMode.ADD_CARD, "Example Store", "", context.getString(R.string.anyDate), context.getString(R.string.never), "0", context.getString(R.string.points), "123456", context.getString(R.string.sameAsCardId), "Aztec", null, null); | |||
| assertEquals(-416706, ((ColorDrawable) activity.findViewById(R.id.thumbnail).getBackground()).getColor()); | |||
| } | |||
|
|
|||
|
|
||
|
|
||
| @Test | ||
| public void longPressOnBarcodeShouldCopyTheBarcodeValue() { |
There was a problem hiding this comment.
This test name isn't completely accurate as it tests both the long and short click flows :)
| public void longPressOnBarcodeShouldCopyTheBarcodeValue() { | |
| public void testCopyBarcodeValue() { |
| String barcodeString = barcodeIdString != null ? barcodeIdString : cardIdString; | ||
| ClipboardManager clipboard = (ClipboardManager) getSystemService(CLIPBOARD_SERVICE); | ||
| if (clipboard != null) { | ||
| android.content.ClipData clip = android.content.ClipData.newPlainText("Barcode", barcodeString); |
There was a problem hiding this comment.
I think most users want to copy the cardId, not the barcodeId.
CardID = the value written on the physical card (mostly the customer number)
BarcodeID = whatever is in the barcode itself (which may contain random gibberish)
The cardId is also what's written in the dialog window.
| String barcodeString = barcodeIdString != null ? barcodeIdString : cardIdString; | |
| ClipboardManager clipboard = (ClipboardManager) getSystemService(CLIPBOARD_SERVICE); | |
| if (clipboard != null) { | |
| android.content.ClipData clip = android.content.ClipData.newPlainText("Barcode", barcodeString); | |
| ClipboardManager clipboard = (ClipboardManager) getSystemService(CLIPBOARD_SERVICE); | |
| if (clipboard != null) { | |
| android.content.ClipData clip = android.content.ClipData.newPlainText("Card ID", loyaltyCard.cardId); |
