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

Buttons should have sizes defined in UIConstants #934

Closed
sblatz opened this Issue Jun 4, 2018 · 5 comments

Comments

Projects
None yet
6 participants
@sblatz
Copy link
Contributor

commented Jun 4, 2018

Buttons are currently hard-coded with heights or use a constant from a different button (rather than naming that constant something more generic). Generic button height types should be defined to reduce confusion.

See file OverlayView

@trungducc

This comment has been minimized.

Copy link
Contributor

commented Dec 10, 2018

I'm not sure but maybe this issue has been fixed because now they are defined as UIConstants.layout.overlayButtonHeight.

@oliviabrown9

This comment has been minimized.

Copy link
Member

commented Dec 10, 2018

We've essentially been fixing as we go, so if we see a button not defined as a constant in a file we're working with, we change it to be a constant. @sblatz to confirm that this is not resolved as there are presumably still many buttons we haven't changed?

@sblatz

This comment has been minimized.

Copy link
Contributor Author

commented Dec 11, 2018

@trungducc There are still cases where buttons have "magic numbers" for their heights. See: SearchSuggestionsPromptView, URLBar, and AddSearchEngineViewController.

@klymenkoo

This comment has been minimized.

Copy link
Contributor

commented Jan 13, 2019

I can look for magic numbers and move them to constants if you don't mind.

@sblatz

This comment has been minimized.

Copy link
Contributor Author

commented Jan 22, 2019

@klymenkoo That'd be great, yeah!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.