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

[IBDesignable] Opt-out designable and inspectable #529

Closed
wants to merge 1 commit into from
Closed

Conversation

jhersh
Copy link
Contributor

@jhersh jhersh commented May 14, 2015

Fixes #460, #510, #520.

@jhersh
Copy link
Contributor Author

jhersh commented May 14, 2015

I don't really like this. Also it is not clear if, when imported as a static library via CocoaPods, you can reliably inject a preprocessor define (TTT_NO_DESIGNABLE) before the pod is compiled.

@jhersh
Copy link
Contributor Author

jhersh commented May 14, 2015

We could define a TTTAttributedLabel subspec that added a preprocessor define, but it would require those users to switch to that subspec.

@cashmash
Copy link

cashmash commented Jul 7, 2015

will that be merged?

@jhersh
Copy link
Contributor Author

jhersh commented Jul 7, 2015

@cashmash probably not in this current form; it is problematic especially when used with CocoaPods. I would welcome suggestions here :)

@Fl0p
Copy link

Fl0p commented Jul 14, 2015

@jhersh you can create additional podspec version only for this fix
something like

  "name": "TTTAttributedLabel",
  "version": "1.13.NO_DESIGNABLE",

pointed on this commit

  "source": {
    "git": "https://github.com/TTTAttributedLabel/TTTAttributedLabel.git",
    "commit": "7e62171a114768725dbf99e10cee83f15f3c98e0"
  },

also with preprocessor flag

"xcconfig": {
    "GCC_PREPROCESSOR_DEFINITIONS": "$(inherited) TTT_NO_DESIGNABLE=1"
  },

so enduser just need to install version without designable like this

pod 'TTTAttributedLabel', '1.13.NO_DESIGNABLE'

@jhersh
Copy link
Contributor Author

jhersh commented Jul 14, 2015

That would require us to maintain two releases in parallel, indefinitely, which is really not sustainable. I don't really like the subspec idea for a similar reason -- it would require everyone to switch. Other suggestions are welcome here.

@Fl0p
Copy link

Fl0p commented Jul 14, 2015

@jhersh
Why maintain ?
Just deprecate it and move forward. IOS9 is coming.
Everybody will use next OS versions with Dynamic Framework linking and swift 2.0

Only few retrogrades will support IOS 3.1.1
And there are version them - 1.13.NO_DESIGNABLE
Until they update theirs project to brand new awesome technologies.
If they want more they always can fork and use theirs own fork and commit.

@jhersh
Copy link
Contributor Author

jhersh commented Jul 14, 2015

One of the hallmark features of this project has been backwards compatibility to iOS 4.3. It's true that won't be maintainable forever but I think we should not be so quick to dismiss it :)

@Fl0p
Copy link

Fl0p commented Jul 14, 2015

Ho Ho Ho
Here the solution that will do everything automatically

  • You need a custom script (bash, ruby or other)
    • this script should scan ../podfile to !use_frameworks keyword (should not be commented)
    • if not found script should use xcodeproj to add TTT_NO_DESIGNABLE=1 macro to Pods-TTTAttributedLabel target
  • than add this script to prepare_command of your spec.

@jhersh thoughts ?

@hagile
Copy link

hagile commented Jul 16, 2015

@Fl0p I'm agree with your thoughts. iOS 9 has been already launched by Apple and now its time to update this awesome library for iOS 7 and greater, send all deprecated methods to the hell. Except few companies who're still supporting iOS 3.1. is just too much as there are not enough devices running this version of iOS. @jhersh, you should really mind our comments and do some hallmark changes in this. Where's @mattt, please share your thoughts as well? If one want to support old versions they can definitely fork and commit their own. For that particular reason, we can't miss super awesome features of new iOS. Waiting for a great update on this.

@jhersh
Copy link
Contributor Author

jhersh commented Jul 16, 2015

There is nothing proposed in this change that requires increasing the minimum deployment target or dropping older iOS SDKs. How is that relevant here?

The specific item under discussion is a mechanism that allows the developer to enable or disable TTTAttributedLabel's IB annotations. Enabling or disabling these annotations must be a conscious decision that the developer makes for each of her projects; @Fl0p's script idea, while interesting, takes that choice away from the developer and is too much 'magic' that will be confusing or unclear to the developer.

We are essentially talking about a compile-time switch using one of these mechanisms:

  1. Add a #define to your project that changes behavior of the label. This is what is currently implemented here in this PR, but it is problematic in CocoaPods to ensure the define is set before the Pods project is compiled
  2. Add a new TTTAttributedLabel subspec, perhaps something like TTTAttributedLabel/NoDesignable, that lets you easily switch in your Podfile between these two configurations
  3. Sidestep the problem entirely by installing TTTAttributedLabel as either a dynamic framework or by dragging the files to your project, effectively marking this as 'wontfix'

I am leaning towards 2. Thoughts?

@codecov-io
Copy link

Current coverage is 83.24%

Merging designable into master will not effect coverage as of b3cbe57

Coverage Diff

@@            master   designable   diff @@
===========================================
  Files            3            3       
  Stmts         1038         1038       
  Branches         0            0       
  Methods          0            0       
===========================================
  Hit            864          864       
  Partial          0            0       
  Missed         174          174       

Powered by Codecov

@jhersh
Copy link
Contributor Author

jhersh commented Jul 16, 2015

Added subspecs as per 2 above. I think this is a workable option.

@getaaron
Copy link
Contributor

@jhersh I think that's the least objectionable proposal I've seen. Pretty cool - I haven't seen xcconfig used within a subspec before.

@jhersh jhersh force-pushed the designable branch 2 times, most recently from a801739 to a904d3f Compare July 16, 2015 19:53
@jhersh jhersh modified the milestone: 2.0.0 Jul 16, 2015
@jhersh
Copy link
Contributor Author

jhersh commented Jul 17, 2015

I've been reconsidering this as of late. This entire mess, and its workaround, applies only to people who (1) use TTTAttributedLabel as a static library and (2) attempt to configure a label in IB. I have to imagine that's a vanishingly small section of the userbase, and this workaround is awfully invasive for such a minority use case. There are other, much more stable workarounds, like using a dynamic framework.

I think it might be simpler and cleaner to instead document this limitation and leave it there.

@jhersh jhersh closed this Jul 18, 2015
@jessesquires jessesquires deleted the designable branch May 10, 2016 16:21
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 this pull request may close these issues.

Usage in IB gives error messages but app works
6 participants