Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use relative layout in widgets #181

Merged
merged 2 commits into from
Feb 25, 2024
Merged

Use relative layout in widgets #181

merged 2 commits into from
Feb 25, 2024

Conversation

newhinton
Copy link
Contributor

This will should fix #127.
It will also make the widget "resizable", so that the internal background color will now stretch to fill the actual size of the widget.

@Bnyro Bnyro requested review from SuhasDissa, Bnyro and M00NJ and removed request for SuhasDissa and Bnyro September 29, 2023 15:41
@newhinton newhinton marked this pull request as draft September 29, 2023 15:44
@newhinton
Copy link
Contributor Author

Okay i found an issue. On my pixel 7 it works fine, but not on the emulator. I think the hardcoded fontsize is the issue.

@Bnyro
Copy link
Member

Bnyro commented Sep 29, 2023

Yes, we should make the font size dynamic depending on the screen size probably, that'll probably work best by using a dimen that differs per resolution. See https://ao.chatoyer.de/questions/32860815/how-to-define-dimens-xml-for-every-different-screen-size-in-android.

@newhinton
Copy link
Contributor Author

I can do that with the dimen-values.

If we define values that way, do we need two layouts per widget?

I would also introduce color to the text, is it okay if i do it in this PR?

@Bnyro
Copy link
Member

Bnyro commented Sep 29, 2023

I can do that with the dimen-values.

If we define values that way, do we need two layouts per widget?

I think it'd work to remove the duplicated widgets and just handle everything through dimens.

I would also introduce color to the text, is it okay if i do it in this PR?

Depends on how it looks, if everyone agrees on it, we'll merge it, otherwise we'll undo the color changes and merge it without. Just do as you like and we'll wee then.

@newhinton
Copy link
Contributor Author

image

This is how the clocks look like now.
I have adjusted the spacing so it usually works.

However, if the user has set the font-size to a large setting, it will break out again. We could fix that by using dp, but i am not sure if that is a good idea. However, this PR should improve the code either way by a bit

@newhinton newhinton marked this pull request as ready for review September 29, 2023 17:18
@Bnyro
Copy link
Member

Bnyro commented Sep 30, 2023

On my device, the widgets look kind of fat and out of place now:

Screenshot_20230930-115034_Trebuchet
Screenshot_20230930-115028_Trebuchet

  1. We should reduce the minimum widget size.
  2. If we don't adjust the font size dynamically as I linked above, this will look kinda ugly on most screen sizes because the font just doesn't fit.

So I think your changes are a good beginning, but we should continue improving responsiveness before merging.

PS: I really like the color changes though!

@M00NJ
Copy link
Member

M00NJ commented Nov 11, 2023

@newhinton are you still working on this?

@newhinton
Copy link
Contributor Author

The issue is that i dont really now how a dynamic font could work. I tried looking at aosp's stock clock, but that is not really useful since it only has a fixed size that only increases borders like this pr currently.

Copy link
Member

@Bnyro Bnyro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, (and sorry for the delay)!

@Bnyro Bnyro merged commit e18f619 into you-apps:main Feb 25, 2024
1 check passed
@Bnyro Bnyro mentioned this pull request Mar 11, 2024
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Digital clock widget wrongly displayed
3 participants