kaandedeoglu / Shark

Swift CLI for strong-typing images, colors, storyboards, fonts and localizations
MIT License
381 stars 15 forks source link

Add namespaces support for Colors #35

Closed bgdn0 closed 2 years ago

bgdn0 commented 3 years ago

Hello,

Is it possible to add nested namespaces support for generated Colors enum (the same way as for Images enum) ?

kaandedeoglu commented 3 years ago

Hey @bgdn0 ,that's a very good suggestion, thanks for getting involved!

I'm a little concerned it'll break code for some people. On the other hand it makes sense to align images & colors. Let's give it a try πŸš€

kaandedeoglu commented 3 years ago

@bgdn0 So the feature is now working, we also get the nesting behavior for colors (it also uncovered a beautiful refactoring that got rid of a lot of duplicate code). That being said, it now feels a little weird for my projects that use Shark. I usually tend to put all colors into a Colors folder, therefore after this change we'll go from:

view.backgroundColor = C.primaryRed

to

view.backgroundColor = C.Colors.primaryRed

So I would like to hear about your use cases and whether you organize color assets per folder when building applications.

bgdn0 commented 3 years ago

Hey @kaandedeoglu , cool, when can I expect the feature release to try it out?

We have separate Asset catalogs for Images and Colors. Colors look something like this (simplified)

Colors.xcassets
    β”œβ”€β”€ button
    β”‚   β”œβ”€β”€ disabled.colorset
    β”‚   β”œβ”€β”€ main.colorset
    β”‚   └── secondary.colorset
    β”œβ”€β”€ input
    β”‚   β”œβ”€β”€ active.colorset
    β”‚   β”œβ”€β”€ error.colorset
    β”‚   β”œβ”€β”€ inactive.colorset
    β”‚   └── placeholder.colorset
    β”œβ”€β”€ list
    β”‚   β”œβ”€β”€ background.colorset
    β”‚   └── separator.colorset
    └── text
        β”œβ”€β”€ error.colorset
        β”œβ”€β”€ main.colorset
        └── secondary.colorset

and the usage

button.backgroundColor = C.button.main
label.textColor = C.text.main

In one project we had colors gray1, ... gray6 and blue1, .... blue4, and it's not obvious should I use gray2 or gray4 for the text, so I prefer to name colors with something more meaningful and use namespaces

kaandedeoglu commented 3 years ago

Hey, the changes are now pushed to the namespaced_color_assets branch, you'll have to build from source to test it:

Simply navigating to the root and doing swift build -c release --disable-sandbox should give you an executable at .build/release/Shark. Happy testing πŸ§€

bgdn0 commented 3 years ago

Works good for me πŸŽ‰. Thank you! πŸ‘

If you don't want to break Colors enum compatibility with the previous version, maybe it worth to add use-namespaces-for-colors as a command line option?

mickeyl commented 2 years ago

Hi @bgdn0. When I started working on Shark with regards to the new framework-specific code generation, I was not aware of the namespaced_color_assets branch. I think it's a good idea to bring this into master before working on the next features. I have just rebased it.

While working on this I noticed one gotcha… the nested namespace enum builder does not evaluate the corresponding Xcode setting yet. This can lead to confusion, hence going forward I'd rather evaluate the Xcode Provides Namespace setting than providing yet another command line option: Bildschirmfoto 2022-09-18 um 14 17 18

I'd also rework the image support then to honor that flag.

What do you think?

mickeyl commented 2 years ago

This is now life as per the 1.6.0 release.