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

Add generation of Table and Collection cell reuseIdentifiers #134

Closed
wants to merge 29 commits into from

Conversation

phatblat
Copy link
Member

@phatblat phatblat commented Jun 6, 2016

Builds on the great work @jmacmullin started in #66. Addresses #19.

Adds collection of all UITableViewCell and UICollectionViewCell reuse identifiers and generation of a new StoryboardCell struct containing an enum of all the identifiers found on each storyboard.

struct StoryboardCell {
  enum Message: String, StoryboardCellType {
    case CustomCollectionViewCellReuseID = "CustomCollectionViewCellReuseID"
    case CustomMessageCellReuseID = "CustomMessageCellReuseID"
    case StandaloneCollectionViewCellReuseID = "StandaloneCollectionViewCellReuseID"
    case StandaloneTableViewCellReuseID = "StandaloneTableViewCellReuseID"
  }
}

Cases handled

  • Storyboards
    • UITableViewCell prototype cells in a UITableView
    • UICollectionViewCell prototype cells in a UICollectionView
    • standalone UITableViewCell associated with a view controller
    • standalone UICollectionViewCell associated with a view controller
  • XIBs
    • UITableViewCell
    • UICollectionViewCell

XIB file parsing has been added. This was trivial to add as the relevant XML is identical between the two Interface Builder file types. However, there are names all over like StoryboardParser and storyboardsCells which now seem weird. Even the storyboards command doesn't seem right now.

API

Extensions to both UITableView and UICollectionView to allow for dequeueing of cells using the SwiftGen generated enum values.

let tableCell = tableView.dequeueReusableCellWithIdentifier(StoryboardCell.StoryboardName.CustomTableCell, forIndexPath: indexPath) as! CustomTableCell

let collectionCell = collectionView.dequeueReusableCellWithReuseIdentifier(StoryboardCell.StoryboardName.CustomCollectionCell, forIndexPath: indexPath) as! CustomCollectionCell

That's a mouthful, but at least it's not using a string to retrieve the cells. As for the as!, that's not very safe. Better to use Reusable for this.

Reusable

The default storyboards template has been updated to generate an extension for every cell that has a customClass and add a reuseIdentifier property with the value found in the storyboard.

extension CustomTableCell { static var reuseIdentifier: String { return "CustomTableCellReuseID" } }
extension CustomCollectionCell { static var reuseIdentifier: String { return "CustomCollectionCellReuseID" } }

Out of Scope

Specifically not included (to be addressed in future PRs)

  • UITableViewHeaderFooterView
  • UICollectionReusableView - need to figure out what to do with elementKind
  • MKAnnotationView 🗺 dequeueReusableAnnotationViewWithIdentifier

WIP

  • Add example to playground
  • Add usage example to readme

@AliSoftware
Copy link
Collaborator

Poll: should we

  • 1️⃣ rename the command from storyboards to ib to match the fact that it now also parses XIBs? (And deprecate the storyboards command progressively?)
  • 2️⃣ Or keep the storyboards command to only parse scenes and segues, and create a separate command (cells?) to parse reuseIdentifiers for Cells?

@@ -93,6 +111,9 @@ struct XCTStoryboardsScene {
return vc
}
}
enum TableViewCell: StoryboardSceneType {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feels strange to conform a cell to some protocol named "…SceneType". Maybe it's time to rename that protocol StoryboardBasedType instead of StoryboardSceneType ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it is a little weird since XIBs don't have "scenes". I think in this case the TableViewCell "scene" should just be suppressed because you can't instantiateInitialViewController from a XIB.

@AliSoftware
Copy link
Collaborator

  • Don't forget to adapt the page in the Playground (or create a new one maybe) to demonstrate that new ability

@AliSoftware AliSoftware modified the milestone: 1.5.0 Jun 6, 2016
case ReuseCellID = "ReuseCellID"
}
enum CollectionViewCell: String, StoryboardCellType {
case XibCollectionViewCellReuseID = "XibCollectionViewCellReuseID"
Copy link
Member Author

@phatblat phatblat Jun 9, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think calling this CollectionViewCell a StoryboardCellType is the weird case as it was found in a XIB. Perhaps a XibCellType that simply extends StoryboardCellType?

@AliSoftware
Copy link
Collaborator

Added the "WIP" label to the PR, don't hesitate to remove it once you consider it done and ready to review for merge 😉

@AliSoftware
Copy link
Collaborator

As for the command, I'm still not settled. Having xibs be an alias of storyboards seems good at first, but I hope people won't think they need to run both swiftgen storyboards and swiftgen xibs to parse each (because if they do they will in fact run the same subcommand twice for nothing), so still open to suggestions…

@phatblat
Copy link
Member Author

Testing this PR out in a real project and ran across a couple issues.

Public

Property 'reuseIdentifier' must be declared public because it matches a requirement in public protocol 'Reusable'

Example:

extension RepoCollectionViewCell { static var reuseIdentifier: String { return "RepoCollectionViewCell" } }

This is an easy fix to the template.

Inheritance

I've got a couple different prototype table cells in a table. They used to both use the same UITableViewCell subclass but part of adopting Reusable included extending that shared class to simply inherit the stored properties, but have a unique type name and reuseIdentifier.

class FileTableViewCell: UITableViewCell, Reusable {
    @IBOutlet var fileNameLabel: UILabel!
    @IBOutlet var gitStatusLabel: UILabel!
    @IBOutlet var dateLabel: UILabel!
}

class FolderTableViewCell: FileTableViewCell {}

Here is are generated extensions which define the reuseIdentifier with the values found in the storyboard.

extension FileTableViewCell { static var reuseIdentifier: String { return "FileTableViewCell" } }
extension FolderTableViewCell { static var reuseIdentifier: String { return "FolderTableViewCell" } }

These are the resulting errors, caused from FolderTableViewCell attempting to override an inherited static property.

Overriding declaration requires an 'override' keyword
Class var overrides a 'final' class var
A declaration cannot be both 'final' and 'dynamic'

Unless I'm missing something, it seems like the only option for this issue is "don't do that". I can easily duplicate the properties in the FolderTableViewCell class, but this could be annoying for a project that did this a lot. This may need to be called out in the docs.

@djbe djbe modified the milestones: Next 3.x, Next minor (4.1.0) Nov 20, 2016
@djbe djbe modified the milestones: Next Minor (4.2.0), Next minor (4.1.0) Jan 1, 2017
@djbe djbe modified the milestones: 4.2.0 (Great Split), 5.0.0 Feb 15, 2017
@djbe djbe modified the milestones: 5.1.0, 5.0.0, Nice to Have May 8, 2017
@bsrz
Copy link

bsrz commented May 9, 2017

This PR is almost a year old. Any plans on merging it in?

@djbe
Copy link
Member

djbe commented May 9, 2017

This is a really old PR, SwiftGen has changed quite a lot since then.
@carlosypunto started a new PR, but it needs more work:
SwiftGen/SwiftGenKit#16

In the meantime, I highly recommend https://github.com/AliSoftware/Reusable.

@AliSoftware
Copy link
Collaborator

AliSoftware commented May 10, 2017

Hi @phatblat

First of all, thanks a lot again for taking the time to create this PR.

I'd like to apologise for letting that feature stale for so long. SwiftGen goals and priorities, including a big refactoring and splitting the project into multiple repositories, took precedence at some point, and we let that PR a bit forgotten, and I'm very sorry for the feeling it might have given to people like you who took the time to contribute.

Given that this PR is quite old and has gotten out of sync with how SwiftGen evolved, I'm gonna close this in favor of SwiftGen/SwiftGenKit#16. We can continue working on this feature in SwiftGenKit from this PR from now on.

To be honest, the next main milestone for SwiftGen and what we're focusing on right now is migrating to a new major version 5.0, removing deprecated template variables and make some cleanup after all that refactoring we did in 4.2. But once it's released (which I hope we won't take too long doing), it'll be time again for adding new feature in this newly cleaned codebase 😉


To thank you for your work and for the time you took to improve the tool, I've invited you to join SwiftGen's GitHub organization's "CoreContributors" team. No pressure to accept nor any obligation in that, don't worry 😉 It will just give you the ability to help triage issues or push directly branches to the repo (instead of your fork)… in case you want to contribute again in the future or help finally make that feature happen 😉

For more information about contributing to SwiftGen in this new GitHub organisation and context, you can read the COMMUNITY.md and CONTRIBUTING.md documents, and if you have any questions don't hesitate to reach out!

@phatblat
Copy link
Member Author

No worries at all :D. This PR is ancient now. I'd love to still contribute to SwiftGen from time to time

@djbe djbe deleted the table-collection-cells branch July 26, 2022 12:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants