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

Remove separator from tableviews #46

Merged
merged 1 commit into from Nov 2, 2016
Merged

Conversation

elland
Copy link
Contributor

@elland elland commented Nov 2, 2016

  • Table view should not be responsible for cell design.

- Table view should not be responsible for cell design.
@elland
Copy link
Contributor Author

elland commented Nov 2, 2016

Merging now so I can move on with work. 🙆

@elland elland merged commit c2b1a77 into master Nov 2, 2016
@elland elland deleted the feature/table-separator branch November 2, 2016 10:37
@3lvis
Copy link
Collaborator

3lvis commented Nov 2, 2016

Sounds like something project specific. Could you move this change to your project instead?

@elland
Copy link
Contributor Author

elland commented Nov 2, 2016

No, this is architecture. Having the tableview be responsible for drawing the cell separator makes no sense. This should've been deprecated years ago, certainly legacy from iPhoneOS still.

@3lvis
Copy link
Collaborator

3lvis commented Nov 2, 2016

I would understand if a lot of people could disagree with you, including @marijnschilling and me. Please roll back.

@elland
Copy link
Contributor Author

elland commented Nov 2, 2016

I don't understand what you mean. 🤔

@3lvis
Copy link
Collaborator

3lvis commented Nov 2, 2016

I'm saying that this is not a good default. And that we should roll back since @marijnschilling and me don't agree with this.

simulator screen shot nov 2 2016 1 13 04 pm

@3lvis
Copy link
Collaborator

3lvis commented Nov 2, 2016

Since this is a change that needs some discussion, I'm going to roll it back so we can discuss it properly.

@3lvis 3lvis restored the feature/table-separator branch November 2, 2016 12:20
@3lvis 3lvis deleted the feature/table-separator branch November 2, 2016 12:21
@marijnschilling
Copy link
Contributor

Where we will discuss it? Here? I have arguments up my sleeve

@elland
Copy link
Contributor Author

elland commented Nov 2, 2016

Here is fine, for posterity. I agree that it's an ugly default, but it is the default. Having the tableview have separators make absolutely no sense to me, it's such a clear violation of responsibilities, but go on.

@marijnschilling
Copy link
Contributor

I think tableView should be responsible for separators because separators say something about how the cells relate to eachother (i.e: last cell doesnt show separator etc.) It's not something a cell can decide for itself.

@elland
Copy link
Contributor Author

elland commented Nov 2, 2016

last cell doesnt show separator

  1. The iOS default has separators even when there are no cells.
  2. Since when? Also you can tell the cell to show or not show an element based on arbitrary logic. Telling what it should display should come from the model/controller, actually laying it out shouldn't. It's the equivalent of having the table view controller setting the cell's title label colour. It's leaking. 🤔

@3lvis
Copy link
Collaborator

3lvis commented Nov 2, 2016

You're proposing to draw the separators inside the cells instead, @elland?

@elland
Copy link
Contributor Author

elland commented Nov 2, 2016

Where else would you draw a cell subview? They're not cell separators, they are decoration inside the cell.

@elland
Copy link
Contributor Author

elland commented Nov 2, 2016

This shows more clearly when you need a decorator that is not supported by the tableview out of the box. Do you subclass the tableview to override the separator drawing or do you set it to .none and add a separator subview to the cell? 🤔

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

3 participants