Skip to content

Conversation

@marcpalmer
Copy link

@marcpalmer marcpalmer commented Sep 5, 2018

This was something I kept wishing the app would do, so I made it.

It's probably hacky/not the way you would do it knowing the code better but it seems reasonable to me e.g. a new CoreData query for this rather than scanning the fetched objects seems overkill given we already have the sessions loaded.

Challenge: try to find a way to scroll to the current session without animating the scroll when the VC first loads. I can't find a time when the tableview has fully laid out for the auto-sizing cells and the wrapping text. Everything I tried meant it scrolled to the wrong location (more noticeable later in the day and the longer the session item names are) - because on first display it still thinks the layout is the estimated size. Help?

@marcpalmer
Copy link
Author

If you like this change we can do the same for My Schedule too.

Copy link

@JoachimK JoachimK left a comment

Choose a reason for hiding this comment

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

Thanks a lot @marcpalmer, that's a feature I also wanted to have!
I left a few ideas for improvements/simplifications, let me know whether these make sense to you. :-)

let session = $0 as! Session
return session == sessionToScrollTo
}) {
tableView.scrollToRow(at: IndexPath(row: rowIndex, section: sectionIndex), at: .top, animated: animated)
Copy link

Choose a reason for hiding this comment

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

Should we be using .top here or the default .none? .none would not scroll it if it's already visible e.g. at the bottom of the screen, it would only make sure that it's visible at all. I'm assuming that this is nicer, as it reduces unnecessary scrolling, but we'd probably have to try it out in the app to see how it feels.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, try it out. I did... it felt good - you just pull down if you wanted to check the previous session but... IMO a very rare case?

}

/// Find the session in the already-fetched results
let nextOrCurrentSession = fetchedResultsController.fetchedObjects?.first { session in
Copy link

Choose a reason for hiding this comment

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

I just noticed that the Session object already has a way to calculate the current and the next Session. Maybe you can use Session.nowSession(forDate:) and Session.nextSession(forDate:) here instead of reimplementing it?

Copy link
Author

Choose a reason for hiding this comment

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

I saw those too, but it seems like a silly idea to run 2 Core Data queries to find this?

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