Pixate / pixate-freestyle-ios

Pixate Freestyle for iOS
Apache License 2.0
848 stars 134 forks source link

Prevented styling if styleMode is PXStylingNone #50

Closed angelolloqui closed 10 years ago

angelolloqui commented 10 years ago

Sometimes I want to disable styling temporary in an item. However, the styler is not checking for the property styleMode before starting the styling. I did not want to modify the preventStyling method but it could potentially be added to every UIView and return this flag by default.

pcolton commented 10 years ago

The loop that actually triggers the styling is already checking for styleMode, so it shouldn't be styling if you set the mode to PXStylingNone. See https://github.com/Pixate/pixate-freestyle-ios/blob/master/src/Styling/Macros/PXStylingMacros.h#L53

Did you find this not to be the case?

angelolloqui commented 10 years ago

Well, for example if you set a text again in a label then it is fired. I have this issue when I want to put a custom attributedString in my label for example, as PX resets the attributed text after updating. My solution is to use this hack together with a PXStylingNone.

angelolloqui commented 10 years ago

Woow, my last commits got inserted into this pull request! Sorry for that!

pcolton commented 10 years ago

And just to be clear, just setting the style mode to none doesn't do the same thing? You need the hack, right?

angelolloqui commented 10 years ago

exactly

pcolton commented 10 years ago

Can you provide some details as to why you want non-recursive updating for the controls you updated?

wprater commented 10 years ago

Im having a similar issue and needed to override -updateStyles as a noop to get my styles to stop for a given view.

angelolloqui commented 10 years ago

@pcolton Why would you want recursive? Setting a text/placeholder should not trigger updates on child elements right? I can imagine that the update is required in the element itself because you could have set a text-transform property, but the childs? what can they change with a text update? maybe I am missing a css selector that depends on texts, but I can not think of any...

Even if most of these elements do not have childs, still some cpu cycles can be saved, specially in UITextViews where it is not so strange to find childs.

pcolton commented 10 years ago

@angelolloqui One of the caveats here is that our virtual controls (what we call them) are also children, but they may not be real views. For example, on UIButton, the 'icon' child is a virtual child that actually sets the image property on a UIButton, but 'icon' itself is not a view or subview in the real sense.

It still might be fine to not recursively style, but I think we'd have to test to see if it exposes any edge cases in which some virtual children don't get styled appropriately.

pcolton commented 10 years ago

I updated the structure of the repo last night. Would you mind merging in the changes then resubmitting the pull request?

pcolton commented 10 years ago

@angelolloqui You can see on issue #84 that not styling children now doesn't style the attributed-text property. Other than some optimizations, can you clarify there were no other reasons NOT to be recursive? That is, I might want to revert that change.