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

Added some UI customization #39

Closed
wants to merge 4 commits into from
Closed

Added some UI customization #39

wants to merge 4 commits into from

Conversation

claesjacobsson
Copy link

  • Subtitle can be used in table.
  • Font and text color can be set for header, footer, cells and detail view.
  • Background color can be set for detail view.

- Subtitle can be used in table.
- Font and text color can be set for header, footer, cells and detail view.
- Background color can be set for detail view.
@claesjacobsson
Copy link
Author

Screenshots of what can be done with my additions:

ios simulator screen shot 17 jun 2015 12 32 01

ios simulator screen shot 17 jun 2015 12 28 24

@vtourraine
Copy link
Owner

Hi Claes, thanks a lot for the PR.

I must say, it looks really nice on your screenshots, but I’m not fond of putting that level of customization into the pod itself. I’ve discussed that in past PR, and my take is that the pod should be simple and straightforward, while keeping the door open to subclassing for advanced customization.

For instance, adding subtitle is a nice touch, but it won’t be useful in the vast majority of cases. On a more technical level, I think you should let the view loading code in loadView, not in the init (see documentation). Last remark, passing a large amount of parameters is always a bit tricky, but if you want to use dictionaries, I’d use one dictionary by label, and keep the UIKit keys for string configuration (à la NSAttributedString).

I hope that doesn’t sound too harsh, I really like seeing people building things on top of this pod. I just don’t think the pod itself needs this kind of addition (everyone always wants something a bit different, obviously). I’ll keep this PR open for a while, see if other people want to join the discussion.

@claesjacobsson
Copy link
Author

Hey Vincent!

Your remarks are definitely valid. My PR was thrown together quite fast to
meet the needs I have in my current project. So I understand if you don't
want to merge the PR.

I will take a step back and see if I can contribute with some more generic
additions.

Thanks!
Claes

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.

None yet

2 participants