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

Refactor typestate configuration into enums #97

Closed
bugadani opened this issue Feb 17, 2021 · 3 comments · Fixed by #116
Closed

Refactor typestate configuration into enums #97

bugadani opened this issue Feb 17, 2021 · 3 comments · Fixed by #116
Milestone

Comments

@bugadani
Copy link
Member

bugadani commented Feb 17, 2021

VerticalOverdraw and HeightMode: #112

@bugadani bugadani added this to the 0.5 milestone Feb 17, 2021
@rfuest
Copy link
Member

rfuest commented Apr 26, 2021

Just to provide some feedback on the API from a user perspective: When I tried to change an example to be vertically centered I wasn't able to do this without looking at the docs. I could easily find the vertical_alignment method in TextBoxStyleBuilder by using code completion, but I had no idea that I could use the same CenterAligned type for both vertical and horizontal alignment.

@bugadani
Copy link
Member Author

bugadani commented Apr 26, 2021

Were the options otherwise easy to discover?

I understand why this can be somewhat difficult and I would like to replace it with a better solution. Originally, I decided on CenterAligned implementing both orientations so that I don't have to use the same name for two different objects, and also avoid having to spell out the module. Maybe a sensible solution would have been to call horizontal CenterAligned and vertical MiddleAligned but I'm not sure. At any rate, refactoring to two distinct enums will fix this.

@rfuest
Copy link
Member

rfuest commented Apr 26, 2021

I only wanted to change this one and haven't paid much attention to the other options. But all options that use types instead of enums will have similar problems. If you decide to keep using type states I would suggest to list the available options in the docs for the corresponding builder methods.

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 a pull request may close this issue.

2 participants