revanced-manager
revanced-manager copied to clipboard
ReVanced
feat: flutter_screenutil integration, dpi responsive ui
Details
This addresses issue #333 and the Reddit post here, fixing the issue of widgets overflowing on low resolution or very high DPI settings. It adds flexibility to a chunk of widgets available, and integrates flutter_screenutil to scale the text when needed.
Some ui elements are too small like button here. Please make sure UI elements are the size before and after your changes when tested on the same device.
That makes sense; I'll adjust that real quick. The hardest part is the Update Manager widget, as it has a very specific design that can't be handled by an ellipsis.
@Ushie is this PR ready to be merged?
Yes and no, it doesn't pass the requirement you have of looking exactly the same, some values could probably be tinkered to achieve that like increase font size a bit, nevertheless I don't have the time for this PR and nobody seems to work on it, it works and if we can compromise on the exact look it's a-okay, just need to sort the merge conflict
Just resolved some conflicts, which caused some errors in the scripts, I'll quickly fix those. I'll also see if I can edit some font sizes to look a bit better.
flutter_screenutil will make UI look bigger in bigger devices and smaller in smaller devices. If device is very small fonts may become unreadable. We should try to make app responsive by updating its layout not by changing the widget size.
My opinion personally, I actually think the opposite. Padding and widget size isn't really being affected, moreso the size of the text itself, but at the moment, the text is too large on those smaller displays, even with layout optimizations (which I did indeed apply some), everything gets cramped quickly, and due to lack of optimization, results in a variety of overflow errors.
However, updating the layout is also a priority of this pull request as well, as not everything can be done with scaling, I'll prioritize that in this next commit.
Finished the said commit. I have tweaked some of the values (along with simplifying some operations, redoing layout optimizations, and bringing it up to speed from the old conflicts), so it should look closer to how one would expect. :^)
I'll admit, I did go a bit overboard with the integration initially, so I have toned down a lot of the dynamic scaling, particularly in areas that don't acutally need it, such as the settings section.
Is there any issue with this commit holding it from getting merged?
As far as I'm aware, not really, I don't know what's stopping it at this point.
If it is to be merged, I'll need to bring it back up to speed by resolving the conflicts (which I can definitely do), but first, I want to make sure that the maintainers are going through with this, since this PR doesn't seem to be of priority.
I'll take a second look when you resolve the conflicts, could you try making it look as close as the existing ReVanced Manager though? if I remember correctly, text sizes were decently smaller last time I checked this out
@BrentBoyMeBob can we have the conflicts resolved and then request for another review?
I'll be on it. I'll also try and make sure it aligns with typical text sizes with some tweaks, as well as the original design.
The old code doesn't quite fit with the current codebase anymore, and had some glaring issues, so I'm currently going to redo it. Will reopen when I'm committing again.
There are still a lot of overflow issues, but I don't know if flutter_screenutil is even necessary after re-evaluating, especially since there have been some changes in cleanliness for the layout since then, and font size isn't really the issue after inspection, it's how the text is handled in relation to the scale that causes them. Plus, it's much easier to manage without since font size is effectively unaffected, so it still will look visually the same on lower DPI devices.
To be honest, a lot of these can be solved with fixes that are conscious to screen size, such as adapting the often noted "Patched Applications" section to some tweaks with LayoutBuilder. I'm on that right now 👍
@Ushie How does this look? I decided to go through with removing flutter_screenutil from the commit entirely, and now it mainly readjusts the layout depending on the room given in widgets, which should mean that devices with conventional screen sizes and DPI should look the same as before, whilst making the needed changes to more zoomed in ones.
@TheAabedKhan Could you please take a look at this PR? it achieves better responsive UI which has been an issue in ReVanced Manager for smaller-sized devices where components end up going out of their containers and unusable
@TheAabedKhan All of your suggestions have worked perfectly it seems, including the larger application_item block, so I've applied them in the newest commit.
Did you test all pages?
- App selector
- Patches selector
- Installer
- Settings
- Dialogs in settings
Did you test all pages?
- App selector
- Patches selector
- Installer
- Settings
- Dialogs in settings
@validcube have you tested the above mentioned pages?
Did you test all pages?
- App selector
- Patches selector
- Installer
- Settings
- Dialogs in settings
@validcube have you tested the above mentioned pages?
Planning to test it today, I'm not at home at the moment
Low and behold, the long awaited merge, thank you lots @BrentBoyMeBob for your patience and I'm glad to see your PR finally ship after such a long time in review, sorry about that!