brarcher / loyalty-card-locker

Stores your barcode-based store/loyalty cards on your phone
GNU General Public License v3.0
171 stars 29 forks source link

Reduce header size in landscape mode #347

Closed TheLastProject closed 4 years ago

TheLastProject commented 4 years ago

Improves #268

Before: photo_2020-01-07_20-35-20 After: photo_2020-01-07_20-35-19

It's a small but good difference, a "start" so to say (Android layout code is very frustrating and I don't feel like spending more time on it).

The code changes may seem very big, but they are in fact pretty small:

The other option was to write code in the LoyaltyCardViewActivity class to check the orientation and make some UI changes. While this would be less code and less (UI) code duplication, I felt patching around in the UI like that when Android provides a native mechanism would be less maintainable, which is why I opted for this way of implementing the change.

If you have any more suggestions on how the landscape UI can be tweaked please mention it, I'm honestly not quite sure what to do while keeping it as pretty as it currently is.

Lucki commented 4 years ago

Not sure how viable this is but here's what Yatse is doing with the big header bar:

He's moving the header bar with the title up while scrolling upwards. When the title reaches the upper limit he's blending the header and the title out and blending the title back in at the upper navigation row.

Pictures While these pictures are taken in portrait mode the same will happen in landscape mode. ![Screenshot_20200202-170728](https://user-images.githubusercontent.com/1408843/73611249-2e4cd700-45e0-11ea-8b90-374401d19fc9.png) ![Screenshot_20200202-170740](https://user-images.githubusercontent.com/1408843/73611248-2db44080-45e0-11ea-83c4-58238c346d46.png) ![Screenshot_20200202-170758](https://user-images.githubusercontent.com/1408843/73611247-2db44080-45e0-11ea-946f-97285ad707be.png)
brarcher commented 4 years ago

The way the app lays out the barcode in landscape I never really liked. The 2D barcodes become really tiny, and the 1D barcodes get really long and are harder to scan. Part of the issue is a lack of knowledge on my part.

I wonder if letting the app be in landscape for display the barcodes is valuable. Having the app go from portrait to landscape was an issue for some, as it could easily happen when handing the phone to a cashier at the store to scan a barcode from the phone. This prompted adding a few features related to "locking" the screen and preventing it from rotating. On my device I have the app configured to disable landscape.

There are also issues with landscape, especially on larger form factor devices (such as tablets). The barcode drawing takes up more memory on larger screens, and has been known to hit OOMs on large tablets. (The drawing code should be improved to not have this issue; again, this is due to a lack of knowledge on my part).

Maybe the simpler approach is to prevent the phone from entering landscape when displaying barcodes. Netfix has a similar approach on Android. Namely, the video list is only viewable as portrait and the video playback only on landscape.

So, is there a good reason for supporting landscape?

TheLastProject commented 4 years ago

I wonder if letting the app be in landscape for display the barcodes is valuable. Having the app go from portrait to landscape was an issue for some, as it could easily happen when handing the phone to a cashier at the store to scan a barcode from the phone. This prompted adding a few features related to "locking" the screen and preventing it from rotating. On my device I have the app configured to disable landscape.

I normally don't like removing features, but I very much agree that it causes more trouble than it ever helps anyone. When I first introduced people to this app many kept accidentally turning it over and over too.

So, is there a good reason for supporting landscape?

Honestly, in my opinion, there isn't. I may take a stab at removing landscape for the barcode view in the future, then we can also remove the locking functionality and so. Would definitely simplify the codebase. Could readd it later, perhaps, we'll see.

Closing this for now because I agree removing landscape for the barcode view makes more sense.