Skip to content

Issues/blog settings cleanup - #4778

Merged
aerych merged 6 commits into
developfrom
issues/blog-settings-cleanup
Feb 5, 2016
Merged

Issues/blog settings cleanup#4778
aerych merged 6 commits into
developfrom
issues/blog-settings-cleanup

Conversation

@aerych

@aerych aerych commented Feb 4, 2016

Copy link
Copy Markdown
Contributor

This is a follow up to #4777 and should wait until #4777 is merged.

Just a bit of house keeping and code style cleanup on the blog settings vc. Blog settings would greatly benefit from a larger refactor to a view model, but that's something for another time.

Needs review: @kurzee (Thanks in advance!)


- (void)tableView:(UITableView *)tableView didSelectRowAtIndexPath:(NSIndexPath *)indexPath
{
[tableView deselectSelectedRowWithAnimation:YES];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Since we add deselectSelectedRowWithAnimation: here it will prevent the default UITableViewController behavior via clearsSelectionOnViewWillAppear. Not a big deal, but general behavior to have a cell stay selected until returning from a detail view.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This was a deliberate change as its absence looked odd, and its addition matches the behavior in most other table views throughout the app.

but general behavior to have a cell stay selected until returning from a detail view.

I wouldn't say this is the general behavior in the app today (based on a quick ack), but maybe it should be. I hadn't noticed the cell highlight dismissing after tapping the back button but now I can't unsee it. :P Let's revert this guy.

@kurzee

kurzee commented Feb 5, 2016

Copy link
Copy Markdown
Contributor

@aerych code looks good minus the cell selection changes. Minor detail, but as you mentioned, this view controller could probably use a larger refactor. :shipit:

@aerych

aerych commented Feb 5, 2016

Copy link
Copy Markdown
Contributor Author

Thanks @kurzee !!

aerych added a commit that referenced this pull request Feb 5, 2016
@aerych
aerych merged commit 5005a1e into develop Feb 5, 2016
@aerych
aerych deleted the issues/blog-settings-cleanup branch February 5, 2016 14:19
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.

2 participants