Skip to content
This repository has been archived by the owner on Aug 14, 2019. It is now read-only.

Added method to scroll to a specific row, optionally animated. #1195

Merged
merged 1 commit into from
Apr 6, 2016
Merged

Added method to scroll to a specific row, optionally animated. #1195

merged 1 commit into from
Apr 6, 2016

Conversation

sebastianludwig
Copy link
Collaborator

This is useful if you want to scroll down to the first unread message for example.

@sebastianludwig
Copy link
Collaborator Author

dunno what Travis is complaining about. Looks badly broken. All 63 unit test run fine locally oO

jessesquires
jessesquires reviewed Sep 12, 2015
View reviewed changes
JSQMessagesViewController/Controllers/JSQMessagesViewController.h
* @param row The index of the row that will be visible
* @param animated Pass `YES` if you want to animate scrolling, `NO` if it should be immediate.
*/
- (void)scrollToRow:(NSInteger)row animated:(BOOL)animated;
Copy link
Owner

Choose a reason for hiding this comment

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

let's use indexPath, more idiomatic. scrollToIndexPath:

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

👍

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Changed it. However I'm not sure if an indexPath is the right fit here. It might be more idiomatic and in line with collection views and table views, but JSQMVC always contains only one section, doesn't it?

@jessesquires
Copy link
Owner

Thanks @sebastianludwig ! A few comments.

@zigdanis
Copy link

zigdanis commented Oct 7, 2015

That would be very nice option!

@alezoffoli
Copy link

I was looking for this too!

@jessesquires jessesquires added this to the 7.2.1 milestone Jan 7, 2016
@jessesquires
Copy link
Owner

Hey @sebastianludwig ! 😄

Sorry for taking so long here. Are you still interested in supporting this PR? Looks like some merge conflicts need to be resolved 😁

@jessesquires
Copy link
Owner

I'm trying to get 7.2.1 out as soon as I can.

@sebastianludwig
Copy link
Collaborator Author

Sure! Gimme a few days.

@jessesquires
Copy link
Owner

@sebastianludwig - more conflicts. I think you're conflicting with yourself from #1203 😆

@jessesquires
Copy link
Owner

@sebastianludwig ping 😄

@sebastianludwig
Copy link
Collaborator Author

^^ Will to to look into it tonight

@jessesquires
Copy link
Owner

Thanks @sebastianludwig ! 😄 🎉 Really appreciate it!

Also -- are you interested in becoming a core contributor?

@codecov-io
Copy link

Current coverage is 66.65%

Merging #1195 into develop will increase coverage by +0.53% as of ceeba3f

@@            develop   #1195   diff @@
=======================================
  Files            63      63       
  Stmts          2294    2309    +15
  Branches          0       0       
  Methods                           
=======================================
+ Hit            1517    1539    +22
  Partial           0       0       
+ Missed          777     770     -7

Review entire Coverage Diff as of ceeba3f

Powered by Codecov. Updated on successful CI builds.

@sebastianludwig
Copy link
Collaborator Author

Sorry, totally forgot about this one.

Core contributor? What would that mean?

@jessesquires jessesquires merged commit 8cba6e5 into jessesquires:develop Apr 6, 2016
@jessesquires
Copy link
Owner

@sebastianludwig send me an email 😄

@eliburke eliburke mentioned this pull request May 23, 2016
Closed
2 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
7.3.0
Development

Successfully merging this pull request may close these issues.
5 participants
@sebastianludwig @jessesquires @zigdanis @alezoffoli @codecov-io