colinta / teacup

This project has been sunset in favor of MotionKit
github.com/motion-kit/motion-kit
Other
602 stars 85 forks source link

Added the ability to "inherit" constraints when extending a style #119

Closed alexrothenberg closed 10 years ago

alexrothenberg commented 11 years ago

If you define these styles

Teacup::Stylesheet.new(:ipad) do
  style :button,
    constraints: [
      constrain_height(22),
      constrain_top(0)
    ]

  style :tall_button, extends: :button,
    constraints: [
      constrain_height(44),
    ]
end

:tall_button will be constrain_height(44) and constrain_top(0)

The logic ensures that only one constraint for an attribute will be applied in the new style

It is not perfect: if you use a symbol in the base style such as :top_left and constraints_top(8) in the extended style both will exist incompatibly.

To get this working I had to make the Teacup merge_defaults module aware of constraints, but I could not think of any other way to do this.

ahmeij commented 10 years ago

+1 This would be very useful to have

colinta commented 10 years ago

Hi @alexrothenberg! I meant to discuss this pull request, but you know how time flies... sorry about the delay - let's get this merged in!

My hesitancy was because this merge makes :constraints a very special value. E.g., if you were applying styles to a non-UIView object, and it had a 'constraints' property, you'd kinda be screwed there... but that's a very strange use case, and I think pragmatism wins out on this one!

On the Symbol issue, we can solve that by resolving the symbol <=> constraint conversion in this new merge_constraints method. Teacup::Constraint methods use symbols in place of UIView instances, so it's safe to convert the symbol into a Teacup::Constraint object at any time.

colinta commented 10 years ago

I added that patch and merged it in!

alexrothenberg commented 10 years ago

@colinta thanks for merging this. I had totally forgotten about this until today.

I like what you did to handle symbols, I don't rememberif I had ignored that or hadn't even thought of it. Probably the later.