SmartPack / SmartPack-Kernel-Manager

Source code of SmartPack-Kernel Manager, the Ultimate Tool to Manage your Kernel
https://play.google.com/store/apps/details?id=com.smartpack.kernelmanager.pro
GNU General Public License v3.0
639 stars 71 forks source link

Layout issue on high resolution screens with Low DPI #88

Closed neekless closed 3 years ago

neekless commented 3 years ago

Hi, I have previously brought this issue up on telegram but I think it's better to open an issue. Thank you for your attempts at fixing the DPI issue on the Overall page! From the telegram group it seems it is fixed for some users. But not for me, and I think I know why.

I see that you've hardcoded the DPI threshold to be 390: if (Utils.getScreenDPI(view.getContext()) < 390) { mParentLayout.setOrientation(LinearLayout.VERTICAL); }

The users that reported it as fixed were using devices with default DPI of 480. Thus when they lower their DPI to 390, things are smaller but still comfortable. For me, using a Pixel 2 XL with default DPI of 560, 390 makes things too small for me. When I set my DPI to my preferred value which is between 440-460, the layout issue is still there.

Screenshots: DPI default 560, single column 560

DPI 380, layout is fine but too small, two columns 380

DPI preferred 450, layout issues, two columns 450

I noticed that the app uses isTablet to determine whether to show one column or two columns. So I suggest instead of hardcoding the DPI threshold to 390, do this instead: if (Utils.isTablet(activity)) { mParentLayout.setOrientation(LinearLayout.VERTICAL); } This way, whenever the app decides to display two columns, the layout will always be correct.

Please let me know what you think. Sorry for such a long issue.

sunilpaulmathew commented 3 years ago

@neekless Thanks for raising the issue. I'm not sure the suggested change will solve the issue (still, there is nothing wrong to try). I realized that the hardcoded DPI check is really necessary to deal with the orientation on devices with low DPI. The real problem is users using non-standard DPI's.

neekless commented 3 years ago

Hi Sunil,

Thanks for trying. If you have a test build, maybe you can let me try.

Also, I'm not sure if the code should be Utils.isTablet(activity) or Utils.isTablet(requireActivity()) You are more familiar. Thank you.

sunilpaulmathew commented 3 years ago

Hi Sunil,

Thanks for trying. If you have a test build, maybe you can let me try.

Also, I'm not sure if the code should be Utils.isTablet(activity) or Utils.isTablet(requireActivity()) You are more familiar. Thank you.

what I mean is, it will most probably fix issues in your device, but led to really strange UI on many other devices (mostly on devices using a low dpi).

neekless commented 3 years ago

Hi Sunil, Thanks for trying. If you have a test build, maybe you can let me try. Also, I'm not sure if the code should be Utils.isTablet(activity) or Utils.isTablet(requireActivity()) You are more familiar. Thank you.

what I mean is, it will most probably fix issues in your device, but led to really strange UI on many other devices (mostly on devices using a low dpi).

On devices with low dpi, if the phone does not have high resolution, then I believe isTablet will be false and the app will show single column, so LinearLayout.VERTICAL will not happen. If the device is high resolution and the user has reduced its dpi low enough such such that isTablet is true, then the app will show 2 columns and LinearLayout.VERTICAL will make the layout correct. If the user did not lower the dpi low enough to make isTablet true, I believe the app will show single column and there will be no layout issues.

But I am just speculating. Maybe you can try a test build in the telegram group and see if users using custom dpi faces any issues. Thank you!

sunilpaulmathew commented 3 years ago

Hi Sunil, Thanks for trying. If you have a test build, maybe you can let me try. Also, I'm not sure if the code should be Utils.isTablet(activity) or Utils.isTablet(requireActivity()) You are more familiar. Thank you.

what I mean is, it will most probably fix issues in your device, but led to really strange UI on many other devices (mostly on devices using a low dpi).

On devices with low dpi, if the phone does not have high resolution, then I believe isTablet will be false and the app will show single column, so LinearLayout.VERTICAL will not happen. If the device is high resolution and the user has reduced its dpi low enough such such that isTablet is true, then the app will show 2 columns and LinearLayout.VERTICAL will make the layout correct. If the user did not lower the dpi low enough to make isTablet true, I believe the app will show single column and there will be no layout issues.

But I am just speculating. Maybe you can try a test build in the telegram group and see if users using custom dpi faces any issues. Thank you!

I already tried the first case. Unfortunately, not only tablet's, layout should be changed to vertical (either checking DPI or a much reliable method) on some other cases also to avoid layout issues on devices using a custom dpi (dpi > 390). A better approach is to add an extra check to make the orientation vertical (something like, isTablet && DPI > 450) in your case.

neekless commented 3 years ago

Ok I understand now. Maybe something like isTablet && DPI > 390 will be good or maybe isTablet && DPI < 480

sunilpaulmathew commented 3 years ago

e something li

Please try this.

neekless commented 3 years ago

https://user-images.githubusercontent.com/22426822/103277055-199cdd00-4a03-11eb-955e-0c23ed8d6ddc.mp4

The layout works!! But the bar with tools and terminal is scrollable.

sunilpaulmathew commented 3 years ago

That's not an issue, I guess. Thanks for testing.

sunilpaulmathew commented 3 years ago

Hopefully fixed with this commit.