Colors: support multiple palettes

[djbe]

Refs SwiftGen/SwiftGenKit#40.

Updates the templates to support multiple color palettes (one for each file).

[AliSoftware]

Only drawback of using a protocol (to support multiple enums) instead of the enum directly is that it's not possible anymore to use the shorthand syntax of the enums (UIColir(named: .title)), as type inference can't work anymore here. you'll have to use the full color names.

Note that we have the same problem with the string template, it's not new. But in the 90% case where there's only one palette seems like a sad situation. Maybe we could improve that later on all templates having that problem? Not sure how though (as using a protocol for cases where we have multiple palettes is still the best option imho)

[djbe]

Well, users can always use the other creation methods, such as ColorName.myRed.color and L10n.Some.Thing.here.

Or we could do some advanced trickery where we check the number of tables/palettes/catalogs/... and not use protocols in those cases.

[AliSoftware]

Yeah the Colors.title.color are still possible but read less nice to me than a constructor. To each their own I guess.

We could indeed use protocols only if multiple palettes and direct enum if only one. Might be an improvement for later.

But in the meantime why do we still have to do Colors.title.color? I thought for strings we switched from enums to functions and constants, shouldn't we do the same here? let c: UIColor = Colors.title ?

[djbe]

Right now I've left it there because each case can have 2 values: a color and a rgbaValue. Should I remove the latter? It would also remove the UIColor(named:).

[AliSoftware]

Ah right. I'm not sure a lot of people query the rgbaValue. Maybe keep this variant with the enum as an alternate template and make the main one direct constants?

[djbe]

Hmmm, we can always add alternative templates if people ask for them, I'd say leave it out for now.

[djbe]

So, the result would be: (swift 2)

enum ColorName {
  /// <span style="display:block;width:3em;height:2em;border:1px solid black;background:#339666"></span>
  /// Alpha: 100% <br/> (0x339666ff)
  static let ArticleBody = Color(rgbaValue: 0x339666ff)
  /// <span style="display:block;width:3em;height:2em;border:1px solid black;background:#ff66cc"></span>
  /// Alpha: 100% <br/> (0xff66ccff)
  static let ArticleFootnote = Color(rgbaValue: 0xff66ccff)
  /// <span style="display:block;width:3em;height:2em;border:1px solid black;background:#33fe66"></span>
  /// Alpha: 100% <br/> (0x33fe66ff)
  static let ArticleTitle = Color(rgbaValue: 0x33fe66ff)
  /// <span style="display:block;width:3em;height:2em;border:1px solid black;background:#ffffff"></span>
  /// Alpha: 80% <br/> (0xffffffcc)
  static let Private = Color(rgbaValue: 0xffffffcc)
}

[AliSoftware]

Btw enum cases should only be used for things you'll likely need to switch over, that was a mistake I did when writing the first templates. We should replace each case x = … by a static let x = ColorStruct(rgbaValue: …) in all the templates we use enum with cases I guess (again, separate PR though. I should stop taking about our of scope ideas inside PRs, damn)

[AliSoftware]

Yup pretty much 👍

[djbe]

Do you want to do this in this PR, or a separate one?

[AliSoftware]

Maybe this can fit in there if it's not too much changes yeah.

[djbe]

Sure thing, on it.

[AliSoftware]

You're gonna hate me be in fact the idea I had was more like this below. Maybe provide this variant as alternate template?

struct ColorName {
  let rgbaValue: UInt32
  var color: Color { return Color.init(named: self) }

  static let ArticleBody = ColorName(rgbaValue: 0x339666ff)
  static let ArticleFootnote = ColorName(rgbaValue:  0xff66ccff)
  …
}
extension Color {
  convenience init(named color: ColorName) {
    self.init(rgbaValue: color.rgbaValue)
  }
}

Usage:

let c1: UIColor = ColorName.ArticleBody.color
let c2 = UIColor(named: .ArticleBody)

Still requires the extra .color on the struct instance to get the actual color, so like the enum solution we had before, except that now it's not an enum but a struct, because there's no point in switch-ing on the colors anyway.

[djbe]

Lemme know if it's ok now, if so, I'll update the documentation samples.

[AliSoftware]

I don't see any unit test containing multiple color palettes in the Expected fixtures here? Did I miss them?

[AliSoftware]

I don't know why I used .init there when I gave you my example, Color(named: self) should work I guess