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

Typing Indicator Support #911

Closed
wants to merge 12 commits into from
Closed

Conversation

nathantannar4
Copy link
Member

This PR adds TypingIndicator support by adding a new supplementary view rather than reserving the last section (as done in #776).

The default typing indicator is an iMessage style typing bubble out of the box but you can also implement your own typing indicator cells!

This implementation is still not complete but is in a functional state to compare against #776.

The goal should be to determine which is a better way of adding this feature, reserving a section, through a supplementary view, or perhaps another method entirely.

@nathantannar4 nathantannar4 added this to the 3.0 milestone Mar 1, 2019
@nathantannar4 nathantannar4 changed the title Typing Indicator Support [Alternative Implementation][In Progress] Typing Indicator Support [In Progress] Mar 1, 2019
@nathantannar4 nathantannar4 changed the base branch from master to 3.0.0-beta March 1, 2019 08:17
@nathantannar4 nathantannar4 changed the title Typing Indicator Support [In Progress] Typing Indicator Support Mar 1, 2019
@ahmedAlmasri
Copy link

ahmedAlmasri commented Mar 22, 2019

@nathantannar4 Hello

i an trying to use setTypingIndicatorViewHidden func but once set is hidden true app crash because the index path 0

Screen Shot 2019-03-22 at 2 15 41 PM

@nathantannar4
Copy link
Member Author

What's the crash?

Sent with GitHawk

@ahmedAlmasri
Copy link

@nathantannar4 check this image

Screen Shot 2019-03-22 at 2 15 41 PM

@nathantannar4
Copy link
Member Author

nathantannar4 commented Mar 22, 2019

The crash is not indicated in the image. Only the line it crashed on.

Please share more information about how you encountered this crash. How many messages did you have? Where do you call setTypingIndicatorHidden?

Sharing just an image of where the crash occurs does not give me enough information to reproduce

Sent with GitHawk

@ahmedAlmasri
Copy link

in the console log do not have any message but look on indexPath value in LLDB expression
0 indices

@nathantannar4
Copy link
Member Author

@ahmedAlmasri The crash is the red error line. You need to expand it. Also, answer my earlier inquires. If i can not reproduce it I cannot work on a solution.

Sent with GitHawk

@nathantannar4
Copy link
Member Author

Let me also be clear, you need messages in your view to show the typing indicator. It cannot be displayed on an empty collection view.

Sent with GitHawk

@ahmedAlmasri
Copy link

look this is steps
if you need full video this

ezgif com-video-to-gif

@nathantannar4
Copy link
Member Author

Where are you calling setTyping from? I see it's in some kind of closure. Are you doing it on the main thread?

Sent with GitHawk

@ahmedAlmasri
Copy link

I am calling setTyping in main thread look the thread stack in my video

@nathantannar4
Copy link
Member Author

Ok, I pushed a guard check on the index path

@ahmedAlmasri
Copy link

I am trying this but show new crash

 Terminating app due to uncaught exception 'CALayerInvalidGeometry', reason: 'CALayer position contains NaN: [nan 0]'
*** First throw call stack:
(
	0   CoreFoundation                      0x000000010ed9e1bb __exceptionPreprocess + 331
	1   libobjc.A.dylib                     0x000000010e33c735 objc_exception_throw + 48
	2   CoreFoundation                      0x000000010ed9e015 +[NSException raise:format:] + 197
	3   QuartzCore                          0x00000001117981a3 _ZN2CA5Layer12set_positionERKNS_4Vec2IdEEb + 141
	4   QuartzCore                          0x0000000111787293 -[CALayer setPosition:] + 57
	5   QuartzCore                          0x00000001117879ef -[CALayer setFrame:] + 561
	6   UIKitCore                           0x000000011355d086 -[UIView(Geometry) setFrame:] + 336
	7   MessageKit                          0x000000010db91088 $S10MessageKit12TypingBubbleC14layoutSubviewsyyF + 3224
	8   MessageKit                          0x000000010db91454 $S10MessageKit12TypingBubbleC14layoutSubviewsyyFTo + 36
	9   UIKitCore                           0x000000011357c795 -[UIView(CALayerDelegate) layoutSublayersOfLayer:] + 1441
	10  QuartzCore                          0x000000011178db19 -[CALayer layoutSublayers] + 175
	11  QuartzCore                          0x00000001117929d3 _ZN2CA5Layer16layout_if_neededEPNS_11TransactionE + 395
	12  UIKitCore                           0x0000000113567077 -[UIView(Hierarchy) layoutBelowIfNeeded] + 1429
	13  UIKitCore                           0x000000011290916d -[UICollectionView _performBatchUpdates:completion:invalidationContext:tentativelyForReordering:animator:] + 244
	14  UIKitCore                           0x0000000112909056 -[UICollectionView _performBatchUpdates:completion:invalidationContext:tentativelyForReordering:] + 90
	15  UIKitCore                           0x0000000112908fd9 -[UICollectionView _performBatchUpdates:completion:invalidationContext:] + 74
	16  UIKitCore                           0x0000000112908f2e -[UICollectionView performBatchUpdates:completion:] + 53
	17  MessageKit                          0x000000010db5ccde $S10MessageKit22MessagesCollectionViewC14scrollToBottom8animatedySb_tF + 318
	18  TestChat                            0x000000010be4b446 $S8TestChat0B10ControllerC11setIsTypingyySbF + 342
	19  TestChat                            0x000000010be4b64f $S8TestChat0B10ControllerCAA0B4ViewA2aDP11setIsTypingyySbFTW + 15
	20  TestChat                            0x000000010be4df1b $S8TestChat0B9PresenterC13observeTyping33_548A160C418CB49B94FB051558E4C4EALLyyFySo15FIRDataSnapshotCcfU_ + 635
	21  TestChat                            0x000000010be4f22c $S8TestChat0B9PresenterC13observeTyping33_548A160C418CB49B94FB051558E4C4EALLyyFySo15FIRDataSnapshotCcfU_TA + 12
	22  TestChat                            0x000000010be41cd2 $SSo15FIRDataSnapshotCIegg_ABIeyBy_TR + 66
	23  TestChat                            0x000000010bf6eeba __43-[FValueEventRegistration fireEvent:queue:]_block_invoke.57 + 122
	24  libdispatch.dylib                   0x00000001106b0595 _dispatch_call_block_and_release + 12
	25  libdispatch.dylib                   0x00000001106b1602 _dispatch_client_callout + 8
	26  libdispatch.dylib                   0x00000001106be99a _dispatch_main_queue_callback_4CF + 1541
	27  CoreFoundation                      0x000000010ed033e9 __CFRUNLOOP_IS_SERVICING_THE_MAIN_DISPATCH_QUEUE__ + 9
	28  CoreFoundation                      0x000000010ecfda76 __CFRunLoopRun + 2342
	29  CoreFoundation                      0x000000010ecfce11 CFRunLoopRunSpecific + 625
	30  GraphicsServices                    0x000000011708d1dd GSEventRunModal + 62
	31  UIKitCore                           0x000000011309281d UIApplicationMain + 140
	32  TestChat                            0x000000010be48487 main + 71
	33  libdyld.dylib                       0x0000000110727575 start + 1
	34  ???                                 0x0000000000000001 0x0 + 1
)
libc++abi.dylib: terminating with uncaught exception of type NSException

@nathantannar4
Copy link
Member Author

Where does that happen?

Sent with GitHawk

@ahmedAlmasri
Copy link

after editing this func

  public func shouldDisplayTypingIndicatorView(at indexPath: IndexPath) -> Bool {
         guard !indexPath.isEmpty else { return false }
        let isLastIndexPath = indexPath.section == messagesCollectionView.numberOfSections - 1
        return isLastIndexPath && !isTypingIndicatorViewHidden
    }

and try the same steps the crash occurs

@nathantannar4
Copy link
Member Author

Ok, sorry this isn't something I can reproduce. I would encourage you to try making some modifications and see what you come up with.

Sent with GitHawk

@nathantannar4
Copy link
Member Author

So @ahmedAlmasri I tried to recreate thus but couldnt. I was using the ChatExample project and turned the MockSocket timer to a 0.1 interval to toggle the indicator as much as possible but had no crash

@nathantannar4
Copy link
Member Author

If you have this project on a public git I would love to check it out and try to debug it

@ahmedAlmasri
Copy link

Ok, i will send my sample project to you, But for your information I am use this branch typingIndicatorView

@nathantannar4 nathantannar4 mentioned this pull request Mar 25, 2019
4 tasks
@peihsendoyle
Copy link

The crash above is on the line:

Screen Shot 2019-03-26 at 1 34 35 PM

@nathantannar4
Copy link
Member Author

This is a curious crash, as the index path that it uses is determined by the collection view 🤔

@nathantannar4
Copy link
Member Author

@peihsendoyle @ahmedAlmasri I made a few changes to when the layout invalidation is called. As this isn't a bug I can reproduce at the moment in the example, if this change does not work can you please:

  • Test if non-animated setTypingIndicatorHidden call works
  • Include a log of the crash, what the value of IndexPath and the MessagesCollectionView are in memory

Thanks! This is definitely an issue we need to fix

@peihsendoyle
Copy link

peihsendoyle commented Mar 26, 2019

@peihsendoyle @ahmedAlmasri I made a few changes to when the layout invalidation is called. As this isn't a bug I can reproduce at the moment in the example, if this change does not work can you please:

  • Test if non-animated setTypingIndicatorHidden call works
  • Include a log of the crash, what the value of IndexPath and the MessagesCollectionView are in memory

Thanks! This is definitely an issue we need to fix

I tried your suggestions but it still doesn't work. It happens just only when I try to hide the indicator view (isHidden = true), it works when showing. Animated property doesn't take effect.

Screen Shot 2019-03-26 at 3 18 12 PM

Screen Shot 2019-03-26 at 3 18 32 PM

Screen Shot 2019-03-26 at 3 18 28 PM

Screen Shot 2019-03-26 at 3 18 26 PM

Screen Shot 2019-03-26 at 3 18 23 PM

Screen Shot 2019-03-26 at 3 18 22 PM

Screen Shot 2019-03-26 at 3 18 19 PM

@peihsendoyle
Copy link

messagesCollectionView log

Screen Shot 2019-03-26 at 3 25 24 PM

@nathantannar4
Copy link
Member Author

When you experience this crash how many messages do you have on screen?

Sent with GitHawk

@nathantannar4
Copy link
Member Author

@peihsendoyle Made changes, please re-test for me

@peihsendoyle
Copy link

It still crashes, but on another line. As you see, the contentBubbleFrame has width = nan??
Screen Shot 2019-03-27 at 3 13 10 PM
Screen Shot 2019-03-27 at 3 13 16 PM
Screen Shot 2019-03-27 at 3 13 23 PM

@nathantannar4
Copy link
Member Author

Is this a project you could share with me so I could debug?

If not please list a more details about the steps you took to produce this. I asked earlier how many messages you had. Anything useful you could share please do, these screenshots really don't help if I can't reproduce the crash because I don't know the state of everything.

Sent with GitHawk

@peihsendoyle
Copy link

okay. give me your bitbucket user, I will temporarily add you to the source code. @nathantannar4

@nathantannar4
Copy link
Member Author

Thank you. My username is nathantannar4

Sent with GitHawk

@peihsendoyle
Copy link

peihsendoyle commented Mar 27, 2019

I’ve shared the repository (with read access) to you recently. Please check the source by yourself. If you have any question, just chat with me on Skype: peihsen_doyle.

Please pull the branch: “origin/feature/typingIndicatorView” (not origin/master)

Thanks and waiting for your solution.

P/s: I’m integrating MessageKit by Cocoapods, and the git is branch “typingIndicatorView”. When you run “pod install”, maybe you will catch some errors within MessageKit, because the branch doesn’t catch up with “3.0.0-beta” branch.

For example, hot fix the cocoapods code:

  • Replace “id” by “senderId” in SenderType.
  • Add senderName : String? and senderId: Int to MessageType.
  • fix some lines in MessageKit after that.

Last but not least, the app need your phone number to login by phone authentication (with verification code sent via sms). Please use these 2 demo account to boost the process because they have conversation together, no need to add friend or create new conversation.

  1. Account named "Nguyen Thi Nghiem":
  • Phone number: +84 903726061 (choose Vietnam on the list to change to +84 prefix)
  • Verification code: 1234
  1. Account named "Hieu Hiep Nguyen":
  • Phone number: +84 367473042 (choose Vietnam on the list to change to +84 prefix)
  • Verification code: 1234

The ChatController.swift is the file you should debug.

@nathantannar4
Copy link
Member Author

@peihsendoyle @ahmedAlmasri I have spent a few hours looking at this and do not understand why its crashing for you. I notice your compiling your code at Swift 4.0 when MessageKit is Swift 4.2 so I am not sure if that has anything to do with it. But I cannot reproduce this in the ChatExample so I am not sure what's going on internally in UIKit, as I have isolated the reason.

I have added a variable that will provide a work around for you both until the core reason can be determined. Set invalidateLayoutOnTypingIndicatorHidden in the layout to TRUE. The reason is below.

/// There is a known issue where the layout invalidation
    /// causes a fatal crash when setting the typing indicator
    /// view to hidden. The cause has been isolated to
    /// `UICollectionViewFlowLayoutInvalidationContext` which
    /// causes an `IndexPath` with 0 indices to be passed into
    /// `layoutAttributesForSupplementaryView` when accessing
    /// `.section`. The current work around is to not use
    /// `invalidateLayout(with: context)` for the case of
    /// setting the typing indicator to hidden but rather
    /// `invalidateLayout()`. This however is not efficent
    /// and thus will not be the default behaviour. Instead,
    /// if you experience the crash set this value to TRUE.
    ///
    /// The default value is FALSE
    public var invalidateLayoutOnTypingIndicatorHidden: Bool = false

@ahmedAlmasri
Copy link

@nathantannar4 I'm using swift 4.2

@ahmedAlmasri
Copy link

@nathantannar4 my sample project sample-chat

@nathantannar4
Copy link
Member Author

Ok clearly this method isn't going to be robust enough. Im going to close this and re-open an updated PR that reserves a section for the typing indicator, similar to #776

@nathantannar4
Copy link
Member Author

@ahmedAlmasri @peihsendoyle Care to review #1038 ? Much appreciated

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.

None yet

3 participants