FluentLayout / Cirrious.FluentLayout

FluentLayout for Xamarin.iOS - sample uses MvvmCross
Microsoft Public License
148 stars 54 forks source link

Update comments on AdvancedFluentLayoutExtensions #33

Closed munkii closed 7 years ago

munkii commented 8 years ago

I meant to submit this PR almost a year ago. When I was first looking at AutoLayout and FluentLayout i found the lack of comments made it harder for me to understand so I thought this might help others

gshackles commented 8 years ago

Thanks for this! I'm certainly open to adding some comments here to help clarify things as long as they're useful and not too intrusive, though part of the API's design is also aimed to try to be self-explanatory as well. One concern I'd have is adding it for just some methods, which could potentially make things more confusing, but that also might not be a big deal.

One nitpick I'd have about this change in particular is the language around create a LayoutConstraint. I think it would be better to speak in terms of our API since we're trying to abstract away the underlying constraint system for the most part.

munkii commented 8 years ago

I get your point about "language" and abstraction. I felt like I wanted to know what was going and so ended up expressing that in the comment. What about "Used to create a constraint", minor change I know but probably enough to demphasize underlying API.

I wasn't sure about the comments on some of the methods. There were some methods I had used a lot so was more comfortable writing the comments for those and thought some comments is better than none

gshackles commented 8 years ago

Yeah, that works

munkii commented 8 years ago

Comments updated

gshackles commented 7 years ago

Going to close this out for now but still happy to consider this again in the future! 😄