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

Media #365

Closed
wants to merge 46 commits into from
Closed

Media #365

wants to merge 46 commits into from

Conversation

piercifani
Copy link

  • Added image display from both a local UIImage and from a remote NSURL
  • Showing and hiding the detail of the image is left to the user.
  • Added an API to show/hide the keyboard

@avently
Copy link

avently commented Jun 22, 2014

Wow! Amazing.
jessesquires, what do you think about it?

@iltercengiz
Copy link

This is really amazing!

Edit: by the way, triggering long press gesture and tapping copy on a media cell causes a crash. Looking for a workaround.

@jessesquires
Copy link
Owner

hello all:

still haven't had a chance to look through this in-depth. just want to let you know i've seen it though. 👍

@piercifani
Copy link
Author

Hey @jessesquires,

If one of the things holding you back is that i've addd SDWebImage as a dependency, the code that uses that pod is pretty much isolated in the JSQMediaHandler class. We could rewrite that class to don't depend on SDWebImage. I added that dependency becauseSDWebImage is the equivalent to AFNetworking on the image downloading/caching scene.

All the best!

@laynemoseley
Copy link

@piercifani Why did you decide to use SDWebImage instead of AFNetworking? Was it because of the offline image caching?

@piercifani
Copy link
Author

@laynemoseley exactly

@robj
Copy link

robj commented Jun 27, 2014

A feature request for this pull request :)

Would be great to be able to easily handle differentiating the two standard cases of media bubble types, that of image and video. In the case of video type, simply overlaying a play button on the bubble.

imessages_play

What do to on tap can be left to #374 didTapMessage:atIndexPath

@ryanthon
Copy link

ryanthon commented Jul 2, 2014

This is awesome! Using it now and it works great! Thanks!

@SebastianBO
Copy link

Nice to hear @tindercylinder, my cocoa pods isn't working at all. So i have to literally copy everything from all the 31 edited files in to the project...

@thieu75
Copy link

thieu75 commented Jul 2, 2014

I'm usine it also. It is great !! Thanks for this amazing work. Kudos !

@piercifani
Copy link
Author

@iltercengiz just updated the component with a fix for the crash you found. Thanks!

@laynemoseley
Copy link

@piercifani Do you plan on adding support for videos media kind?

@piercifani
Copy link
Author

@laynemoseley it's not in my backlog since I don't need them. However, adding support would be fairly simple, just adding two more kinds to JSQMessageKind : JSQMessageLocalVideoMediaKind and JSQMessageLocalVideoMediaKind. After that, edit the xib to add/remove a "Play" image according to the media kind.

@ezrasuki
Copy link

@piercifani sorry for the newbie question, but is there a simple way to use your branch using Cocoapods?

@VincentSit
Copy link

@ezrasuki - It's very easy to do.

pod 'JSQMessagesViewController', :git => '[email protected]:piercifani/JSQMessagesViewController.git', :branch => 'media'

@ezrasuki
Copy link

@VincentSit Thanks I tried and it works fine but I couldn't get some of the features to work because the code I pulled from Cocoapods didn't contain some of the up-to-date commits mentioned at the top of this page, for example this: https://github.com/piercifani/JSQMessagesViewController/blob/dde28045f287b3bec52c422fc4923f2754676b3e/JSQMessagesViewController/Controllers/JSQMessagesViewController.m

Do you know how I can use the code that has the updated version?

@VincentSit
Copy link

@ezrasuki Try using the following code to use the latest commit:

pod 'JSQMessagesViewController', :git => '[email protected]:piercifani/JSQMessagesViewController.git', :commit => '4f764df0c9fcb19b3fd1c923cbde3270c4610345'

@thieu75
Copy link

thieu75 commented Aug 25, 2014

Hi guys,

Great job !

I think I have found a bug.

I have try to deactivate the PressLong gesture recognizer when the messageData is kind of Media.
So I added the method - (void)jsq_handleLongPressGesture:(UILongPressGestureRecognizer *)longPress in JSQMessagesCollectionViewCellIncomingMedia (and outgoing).

But I have discovered that with a text cell and despite the right cellIdentifier (outgoingCellIdentifier) passed in - (UICollectionViewCell *)collectionView:(JSQMessagesCollectionView *)collectionView cellForItemAtIndexPath:(NSIndexPath *)indexPath, the text cell will become a media cell
// JSQMessageViewController

Very Strange behavior.
To reproduce it just add (void)jsq_handleLongPressGesture in JSQMessagesCollectionViewCellOutcomingMedia and do a long press on a Text cell. It will be triggered.

Any idea ? I didn't manage to find a workaround.

Thanks,
Matthieu

EDIT : I have found my problem. The outgoing.xib pointed to the media ! No more problem now.

Pierluigi Cifani and others added 6 commits August 26, 2014 11:22
Conflicts:
	JSQMessagesViewController/Controllers/JSQMessagesViewController.m
	JSQMessagesViewController/Views/JSQMessagesCollectionView.h
	JSQMessagesViewController/Views/JSQMessagesCollectionViewCell.h
	JSQMessagesViewController/Views/JSQMessagesCollectionViewCell.m
Conflicts:
	JSQMessagesViewController/Controllers/JSQMessagesKeyboardController.m
	JSQMessagesViewController/Controllers/JSQMessagesViewController.m
@Antairez
Copy link

Antairez commented Sep 8, 2014

@piercifani Thank you for the pull request, the media branch is working good besides a small thing, I've found a bug that I believe has to to with string size calculation, using your media branch I found out that chinese characters get cut off when they are more than 2 lines, like this:
screen shot 2014-09-07 at 9 22 27 pm
while testing on the latest master branch from Jesse, the problem is gone (I believe its 5.3.1):
screen shot 2014-09-07 at 9 24 15 pm

Do you have any insights on this? Thank you.

updated

The problem lies under - (CGSize)messageBubbleSizeForItemAtIndexPath:(NSIndexPath *)indexPath, I replaced the stringSize calculation by Jesse's code and now the characters are displayed correctly.

CGRect stringRect = [[messageData text] boundingRectWithSize:CGSizeMake(maximumTextWidth - textInsetsTotal, CGFLOAT_MAX)
                                                                 options:(NSStringDrawingUsesLineFragmentOrigin | NSStringDrawingUsesFontLeading)
                                                              attributes:@{ NSFontAttributeName : self.messageBubbleFont }
                                                                 context:nil];

            CGSize stringSize = CGRectIntegral(stringRect).size;

@piercifani
Copy link
Author

Hey @Antairez. Yes, that'll work but it will be broken for custom fonts. Check issue #347 for more info

@Antairez
Copy link

Antairez commented Sep 8, 2014

@piercifani I see, it looks like a tradeoff we will have to make right here. Nevertheless I found your PR very helpful, thank you for sharing.

@Antairez
Copy link

Antairez commented Sep 8, 2014

Hi @piercifani , I am trying to fix #264 here, I tried to do a manual merge on your JSQMessageCollectionViewCellIncomingMedia.xib and JSQMessageCollectionViewCellOutgoingMedia.xib based on 8a616d4?, but the commit has some slight different constraints than the ones you have here, and I don't know which one I should be changing. Can you help me here?

@jessesquires
Copy link
Owner

Hello all -- I've implemented media messages on the release-6.0 branch. Still not 100% complete, but almost. More details at #476 and #223.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.