Move barcode to top on tap #348
Conversation
|
This also works when rotating screens? Is there a setting to always have it at the edge of the screen? |
Yeah, but it doesn't fix the barcode sizing issues that sometimes happen in landscape view.
No, because it really hides everything, also the edit button. So letting this be a default would make a lot of features unreachable. It's just a single tap away, though. |
|
From quick scan of code (I'm not familiar with the code base) and testing debug build provided for #143 this looks good to me 👍 For the other questions about about additional features - those would be nice to have but I see those as new enhancements (there are issues open for some of those) that could be done in a different PR. |
|
@TheLastProject, I'm wondering about the barcode entering the status bar. For the 2D barcodes could the app icons interfere with the barcode squares and prevent proper scanning? Should the barcode be below the status bar to prevent scanning issues? |
|
The status bar is actually hidden, but Android force-shows it whenever I try to take a screenshot, so I couldn't show it is hidden normally. |
There was a problem hiding this comment.
Reviewed 1 of 2 files at r1.
Reviewable status: 1 of 2 files reviewed, 3 unresolved discussions (waiting on @TheLastProject)
app/src/main/java/protect/card_locker/LoyaltyCardViewActivity.java, line 150 at r1 (raw file):
if(barcodeIsFullscreen) { // Properly reset state to prevent any issues
Like what issues?
app/src/main/java/protect/card_locker/LoyaltyCardViewActivity.java, line 366 at r1 (raw file):
} private void setBarcodeFullscreen(boolean enabled)
Small nit, but the argument 'enabled' should be in present tense as it is a request. That is, 'enable' would be better. 'setFullscreen' may be better, as the if() statements below may be easier to understand.
app/src/main/java/protect/card_locker/LoyaltyCardViewActivity.java, line 366 at r1 (raw file):
} private void setBarcodeFullscreen(boolean enabled)
Can there be a comment on this function, describing what it does.
Oh, I see. You are right. Trying that out on an emulator it is working just fine. Neat. (: I wonder how a user will discover this feature. (Or almost any feature in the all, really...). Do you think (in a separate PR) adding a new entry to the first start wizard would help? I'm not that familiar with how to help users discover features in apps. Nothing that needs to be solved now, just a thought. |
|
The issue with the first start wizard is that existing users probably won't see it and thus still not know about the new features. Maybe it'll be interesting to add something like https://steemit.com/utopian-io/@andrixyz/using-material-showcase-view-to-tutor-users-in-android-app-development, so we can highlight interesting new features on new releases and guide new users through how to use the app, replacing the first start wizard entirely with such hints? |
I can confirm this based on testing my device. |
There was a problem hiding this comment.
Reviewable status: 1 of 2 files reviewed, 3 unresolved discussions (waiting on @brarcher)
app/src/main/java/protect/card_locker/LoyaltyCardViewActivity.java, line 150 at r1 (raw file):
Previously, brarcher (Branden Archer) wrote…
Like what issues?
For some reason, when resuming while fullscreen mode is enabled, the barcode is stretched out to fit the whole screen. I couldn't find another way to resolve this. I've improved the comment. Maybe this is worth creating a new issue for so we can at some point not reset state when resuming, but at least can have this feature now?
app/src/main/java/protect/card_locker/LoyaltyCardViewActivity.java, line 366 at r1 (raw file):
Previously, brarcher (Branden Archer) wrote…
Small nit, but the argument 'enabled' should be in present tense as it is a request. That is, 'enable' would be better. 'setFullscreen' may be better, as the if() statements below may be easier to understand.
Done.
app/src/main/java/protect/card_locker/LoyaltyCardViewActivity.java, line 366 at r1 (raw file):
Previously, brarcher (Branden Archer) wrote…
Can there be a comment on this function, describing what it does.
Done.
TheLastProject
force-pushed
the
feature/fullscreenBarcode
branch
from
3d89569
to
fd6c66a
Compare
There was a problem hiding this comment.
Maybe it'll be interesting to add something like https://steemit.com/utopian-io/@andrixyz/using-material-showcase-view-to-tutor-users-in-android-app-development,
That does seem interesting. Maybe a future improvement for another PR. (:
Reviewed 1 of 1 files at r3.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @TheLastProject)
app/src/main/java/protect/card_locker/LoyaltyCardViewActivity.java, line 150 at r1 (raw file):
Previously, TheLastProject (Sylvia van Os) wrote…
For some reason, when resuming while fullscreen mode is enabled, the barcode is stretched out to fit the whole screen. I couldn't find another way to resolve this. I've improved the comment. Maybe this is worth creating a new issue for so we can at some point not reset state when resuming, but at least can have this feature now?
Making a new issue for it and improving it later sounds find. Can you take care of creating the new issue?
There was a problem hiding this comment.
Reviewable status:
complete! all files reviewed, all discussions resolved
On a tap of the barcode, moves the barcode to the top of the screen. On another tap, move things back to normal. On the second screenshot, the top bar is normally hidden, it is only visible because creating a screenshot makes it reappear. Fixes #143.
This change is