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

Allows use type of cellView directly #49

Merged
merged 1 commit into from
Jun 15, 2016

Conversation

encero
Copy link
Contributor

@encero encero commented Jun 15, 2016

Uses enum JTAppleCalendarViewSource as definition of class or Xib for JTAppleDayCell subview.

As bonus allows register class for DayCell directly with type instead of string

calendar.registerCellViewClass(cellClass: CellView.self)

@patchthecode please take a look, and tell me what are you thinking.

I think this simplifies JTAppleDayCell#setupCellView code. But i really just want to register cells with it's type instead of xib or string name of class.

PS: I sucks on naming things so please rename anything to your liking.

… JTAppleDayCell subview. Allows use type of cellView directly as calendar.registerCellViewClass(cellClass: CellView.self)
@patchthecode
Copy link
Owner

Hey, thanks for the pull request. Some notes below:

  1. func xibFileValid() -> Bool - This looks good. You have even added // todo: check other source cases. I had forgotten to check for the other sources in there. Big thanks.

  2. I was thinking we can place this inside the JTAppleDayCell.swift file instead of creating a new file for this?

 enum JTAppleCallendarCellViewSource {
    case fromXib(String)
    case fromType(AnyClass)
    case fromClassName(String)
}
  1. Your code made the one i had neater, and it works correctly. But can you let me know what is the benefit of adding registration by type? Or can you help me understand why you had to register by class type in your case?

@encero
Copy link
Contributor Author

encero commented Jun 15, 2016

Hi,

  1. In my opinion there isn't reason to do full check in xibFileValid(), just check that there is some source set. So i don't write implementation for other sources

  2. Sure. It's only my coding style to have one class/enum for one file

  3. I don't use nib files in my projects ( to be honest i don't know how to use one :) ). So i need convenient way to use plain classes. And registering by class is what UITableView and other cell based view does in UIKit

And of course Swift compiler will check for typos when using classes instead of plain strings

@patchthecode
Copy link
Owner

Ok. youre right. Swift compiler will check for typos.
Great code. I'll do a merge.

Only one thing I will change --> I'll put the enum in the cell class so that things do not get get confusing for me. I may follow your convention in future and move the enums to a single file. But for now, it makes things easier for me with less files.

Big Thanks!!

@patchthecode patchthecode merged commit 12ae6de into patchthecode:master Jun 15, 2016
patchthecode added a commit that referenced this pull request Jun 15, 2016
Note: I may move enums to separate files in future to keep code neater.
Suggestion from user @encero
#49
@patchthecode
Copy link
Owner

Done. Committed and pushed. I also left a note on that commit. I will make the enums neater later on. I'm not 100% sure that I will put each in a separate file yet because (i think) more files will be confusing for me. But I will make them neater. Thanks again.

@patchthecode
Copy link
Owner

Also if you like the library. Leave a Star ★ rating on github :)
It's a very young project, but i believe that it is the best iOS calendar on github. It just needs to be more popular. It is sad I do not know of more ways to make it more popular.

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.

2 participants