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

Replace explicit DPI conversions with standard unit type #416

Closed
oshorefueled opened this issue May 17, 2021 · 0 comments · Fixed by #418
Closed

Replace explicit DPI conversions with standard unit type #416

oshorefueled opened this issue May 17, 2021 · 0 comments · Fixed by #418
Assignees
Labels
enhancement New feature or request
Projects

Comments

@oshorefueled
Copy link
Contributor

running $ grep "\.PxPer

returns a few instances of explicit use of the pixel-to-dp scale. The pageCommon.layoutNavDrawer converts from Dps to pixels:

gtx.Constraints.Min.X = int(gtx.Metric.PxPerDp) * width

However, PxPerDp may not be integral; a better version is

gtx.Constraints.Min.X = int(gtx.Metric.PxPerDp * width)

Even better is to change the type of width, navDrawerWidth, navDrawerMinimizedWidth to unit.Value, and use

gtx.Constraints.Min.X = gtx.Px(width)

A related issue is the godcr/ui/decredmaterial.pxf function; it is a copy of gioui.org/unit.Metric.Px except that it special-cases the zero scale. I think it is better to never construct zero-valued unit.Metrics and use the standard Px method instead.

This is in reference to Elias' review: https://paste.sr.ht/~eliasnaur/cea1d29d6a5f96668b5e166c2f39ef596974574f

@oshorefueled oshorefueled added the enhancement New feature or request label May 17, 2021
@oshorefueled oshorefueled added this to To do in godcr board via automation May 17, 2021
godcr board automation moved this from To do to Done May 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
godcr board
  
Done
Development

Successfully merging a pull request may close this issue.

2 participants