jessesquires / JSQMessagesViewController

An elegant messages UI library for iOS
https://www.jessesquires.com/blog/officially-deprecating-jsqmessagesviewcontroller/
Other
11.14k stars 2.82k forks source link

iPhone X Issues Megathread #2179

Closed chrisballinger closed 5 years ago

chrisballinger commented 6 years ago

Although this library is deprecated, there are a large number of people still using it in production who will probably all be rushing to solve iPhone X layout issues over the next few weeks. Since the network graph is too large to display on GitHub, it's hard to see what other people have done to address these issues. As the legacy maintainer I want to make sure we can still maintain a dialogue for certain critical bugs.

The main issue that I've found is that the constraints for the input toolbar needed to be adjusted for the "safe area" (see example below). This can be solved by adding "safe area" support to the xib file, but it requires a deployment target of iOS 9 or higher. I also noticed that although it solved the issue in the demo project, it was still broken in my own project (and still haven't figured out why), so your milage may vary. (just kidding the old xib was cached)

I'll soon be creating a new iphone_x branch containing my fixes so far. Feel free to discuss your solutions here, but please keep conversation constructive and limited to iPhone X issues only.

Additional Reading:

Before:

screen shot 2017-11-01 at 2 22 54 pm

After:

screen shot 2017-11-01 at 2 25 47 pm
chrisballinger commented 6 years ago

Feel free to cherry-pick this commit: https://github.com/ChatSecure/JSQMessagesViewController/commit/bec4c848bb59c65ae040e1c31f2e8052b97657e2

otymartin commented 6 years ago

Beautiful, thanks for this @chrisballinger and really good idea opening a consolidated iPhone X issue page. @chrisballinger question how did you solve safe-area guide issue in your own project? You mentioned the old xib being cached, I can't seem to solve it in the demo or my own project.

Update This persons answer is the only solution that worked for me. Placing this code in JSQMessagesInputToolbar.m

-(void) didMoveToWindow{
[super didMoveToWindow];
 if (@available(iOS 11.0, *)) {
     [[self bottomAnchor] constraintLessThanOrEqualToSystemSpacingBelowAnchor:self.window.safeAreaLayoutGuide.bottomAnchor multiplier:1.0].active = YES;
     }
}

https://stackoverflow.com/questions/46439975/jsqmessageviewcontroller-ios11-toolbar

chrisballinger commented 6 years ago

@otymartin I was able to get it working with a clean build using just the xib modifications in my patch. Your patch probably works better for those that still need iOS 7/8 support.

otymartin commented 6 years ago

@chrisballinger curious if you get the issue where theres clear space between the bottom of the toolbar and the bottom of the screen allowing message bubbles to be seen through? like so. Cant seem to solve it.

screen shot 2017-11-02 at 10 29 45 pm
chrisballinger commented 6 years ago

Hmmm no but I realized that the constraints for the xib method only need to be pinned to the bottom safe area. (the translucent bleed through in the navbar is clipped at the moment)

otymartin commented 6 years ago

@chrisballinger question are you using the master or develop branch?

eliburke commented 6 years ago

@chrisballinger I cherry picked your bec4c84 commit for JSQMessagesViewController.xib and it fixed the bottom margin.

I also needed to use @lucytheslayer's fix from #2172 to fix the top cell underlapping the Nav Bar.

// Swift
if #available(iOS 11.0, *){
    self.collectionView.contentInsetAdjustmentBehavior = .never 
    self.collectionView.contentInset = UIEdgeInsetsMake(64, 0, 0, 0)
    self.collectionView.scrollIndicatorInsets = self.collectionView.contentInset 
}

// ObjC
if (@available(iOS 11.0, *)) {
    self.collectionView.contentInsetAdjustmentBehavior = UIScrollViewContentInsetAdjustmentNever;
    self.collectionView.contentInset = UIEdgeInsetsMake(64, 0, 0, 0);
    self.collectionView.scrollIndicatorInsets = self.collectionView.contentInset;
}

The final change I had to make was to set self.inputToolbar.barTintColor to match self.inputToolbar.contentView.backgroundColor. Without it, the home indicator was choosing a gray shade that matches nothing else in my app.

chrisballinger commented 6 years ago

@otymartin I maintain my own fork that's mostly 7.3.4 with some cherry-picks from develop and a few other small modifications. At one point a while ago I tried to migrate my project to 8.0.0/develop and had a number of issues so I had to revert. I'm not sure what to do with the current state of upstream because IMO both the master branch and develop are broken in their own ways.

@eliburke Thanks for that tip! Is there a way to make that 64 not just a magic number?

SD10 commented 6 years ago

@eliburke I'm not sure #2172 is a fix because the contentInset is hard coded. It also needs to consider nav bar variations + JSQ's additionalContentInset property which has a corresponding set method.

eliburke commented 6 years ago

Good point. I was rushing and didn’t pay attention to anything besides the fact that it appeared to fix the (obvious) underlap.

Worse than magic values, it looks to be wrong on iPhone X— there is too much inset when the keyboard is closed, and it still underlaps when open.

Tulleb commented 6 years ago

I am proposing a fixed fork based on the JSQ latest develop branch commit.

It is using the didMoveToWindow solution. Not ideal but worth to try while waiting for Apple's answer about inputAccessoryView's safe area layout guide attachment, or any other better fix.

You can add this to your Podfile, replacing the previous JSQ line:

pod 'JSQMessagesViewController', :git => 'https://github.com/Tulleb/JSQMessagesViewController.git', :branch => 'develop', :inhibit_warnings => true
chrisballinger commented 6 years ago

@Tulleb I don't mind merging the right fix(es) into upstream/develop once they're ready.

Tulleb commented 6 years ago

I spent my evening on it yesterday. I gave up after a few hours and ended to use the didMoveWindow solution.

The problem is that the inputToolbar is not great to use as an inputAccessoryView. In my opinion we would need to convert it into a UIView in order to fix it correctly (instead of a UIToolbar at the moment). But from what I read, it seems to be a problem from Apple's side. So maybe a fix would arrive in a further iOS version.

@chrisballinger do you want me to open a PR with the didMoveToWindow feature while we get something better?

alessiettojr commented 6 years ago

Hello everyone, I'm new to the game and wanted to know if there was a guide to fixing the view problem for iphone X. Thank you all

SD10 commented 6 years ago

@Tulleb didMoveToWindow is what we decided to use at MessageKit as well. I think this is becoming a widely used solution, have had issues trying to solve this problem other ways.

Tulleb commented 6 years ago

@SD10 Didn't you try to set up a UIView as the inputAccessoryView in the first place? I don't understand why it absolutely has to be a UIToolbar. I'm not sure this is the expected behaviour from Apple for this element.

Tulleb commented 6 years ago

@alessiettojr You will find your answer in the previous messages of this thread.

alessiettojr commented 6 years ago

@otymartin perfect runs smoothly, thank you very much

SD10 commented 6 years ago

@Tulleb You're right, I forgot about the UIToolbar in JSQMVC. Yes, we decided to go with a UIView from the start to get the flexibility we wanted

J-Bossi commented 6 years ago

For me an extension to the JSQMessagesInputToolbar works

extension JSQMessagesInputToolbar {
   override open func didMoveToWindow() {
     super.didMoveToWindow()
     if #available(iOS 11.0, *) {
       if self.window?.safeAreaLayoutGuide != nil {
         self.bottomAnchor.constraintLessThanOrEqualToSystemSpacingBelow((self.window?.safeAreaLayoutGuide.bottomAnchor)!, multiplier: 1.0).activate()
       }
     }
   }
 }

https://stackoverflow.com/questions/46886345/jsqmessagesviewcontroller-inputtoolbar-ios-11-iphone-x-swift4/46971214#46971214

alanscarpa commented 6 years ago

Using the extension provided by @J-Bossi fixes the input toolbar initial position, but now the most recent message is partially covered by the input toolbar. Anyone have experience with this?

dmathewwws commented 6 years ago

@alanscarpa or @otymartin did you find an answer to your question? im running into the same problem

nbonamy commented 6 years ago

Hello,

I am also facing a different issue: once the keyboard has popped out, if you close it (by swiping down) for instance then the very bottom part gets transparent:

screen shot 2018-03-15 at 18 58 24

Any one has a clue how to fix this?

Thanks, Nicolas

Tulleb commented 6 years ago

pod 'JSQMessagesViewController', :git => 'https://github.com/Tulleb/JSQMessagesViewController.git', :branch => 'develop', :inhibit_warnings => true

otymartin commented 6 years ago

@Tulleb does this fix the transparent keyboard issue @nbonamy mentioned above? curious

nbonamy commented 6 years ago

@Tulleb I was already using your fork although not on the 'develop' branch. Unfortunately switching to 'develop' does not fix the issue I am mentionning. The area below the keyboard is OK on view controller display but if you open the keyboard and then close it (by swiping down or posting a message), the area becomes transparent.

otymartin commented 6 years ago

@nbonamy @Tulleb I noticed behaviour that causes and removes the transparency. When you type a long enough message that forces the textView to expand to 2 or more lines, the transparancy issue arises. Do the same thing, compose a message with more than 2 lines (textview must expand) but this time press send while the keyboard is resigned it resets the layout and transparency is gone. So I dont know objective C meaning i cant trace it but whoever does should be able to trace whatever causes the layout glitch when textView expands to more than 2 lines

nbonamy commented 6 years ago

@otymartin @Tulleb in my case it does not even have to expand. just pop-out the keyboard and then slide the collection view down to collapse it. I tried some fixes in - (void)textViewDidEndEditing:(UITextView *)textView but did not succeed.

Tulleb commented 6 years ago

True, sorry for the disappointment. In my case, I just need to send one message and then pop back: so the glitch isn't a real issue...

I guess it's time to move on to @SD10's https://github.com/MessageKit/MessageKit.

SD10 commented 6 years ago

I'm pretty sure we solved this issue but we have enough existing problems of our own. JSQMVC is like 4x faster than MessageKit. This doesn't matter all too much if you're managing your data source reasonably, however, I'm still considering a rollback to an API that mirrors JSQMVC 😕

otymartin commented 6 years ago

@SD10 you solved in for JSQMVC or MessageKit? Im just holding onto JSQMVC until MessageKit is ready

nbonamy commented 6 years ago

<warning><hack class="ugly"> Finally "solved" this by adding an opaque UIView that I position at the bottom of the screen in the case the device is an iPhone X. Will do for the moment (once I add device rotation handling...). </hack></warning>

SD10 commented 6 years ago

Sorry for the lack of clarity @otymartin. Meant MessageKit

Luten commented 6 years ago

@nbonamy Found a solution without empty space below input toolbar, the idea that not the whole toolbar should be above the safe area, but only its content:

-(void) didMoveToWindow{
    [super didMoveToWindow];
    if (@available(iOS 11.0, *)) {
        if (self.window.safeAreaLayoutGuide != nil) {
            [[self.contentView bottomAnchor] constraintLessThanOrEqualToSystemSpacingBelowAnchor:self.window.safeAreaLayoutGuide.bottomAnchor multiplier:1.0].active = YES;
        }
    }
}
Ariandr commented 6 years ago

@Luten I created a fork and implemented that solution which fixes empty space. It's based on the release 7.3.5.

Usage in Podfile:

pod 'JSQMessagesViewController', :git => 'https://github.com/Ariandr/JSQMessagesViewController.git', :branch => 'master', :inhibit_warnings => true

otymartin commented 6 years ago

@Luten @Ariandr I tried that solution.

extension JSQMessagesInputToolbar {

    open override func didMoveToWindow() {
        super.didMoveToWindow()
        guard #available(iOS 11.0, *), App.shared.isiPhoneX, let window = self.window else { return }
        self.contentView.bottomAnchor.constraintLessThanOrEqualToSystemSpacingBelow(window.safeAreaLayoutGuide.bottomAnchor, multiplier: 1.0).isActive = true
    }
}

But the input textbox layout breaks atleast in simulator. Initially its height is like at zero, and expands as you add text.

screen shot 2018-05-10 at 12 53 46 pm screen shot 2018-05-10 at 12 54 06 pm

Ariandr commented 6 years ago

@otymartin I think I know your issue. My solution (in fork) is built on the top of the initial solution, not only using that code.

This can be solved by adding "safe area" support to the xib file, but it requires a deployment target of iOS 9 or higher.

I updated .xib file and target to 9.0. And added that piece of code. So, feel free to install my custom pod and look at it. :)

pod 'JSQMessagesViewController', :git => 'https://github.com/Ariandr/JSQMessagesViewController.git', :branch => 'master', :inhibit_warnings => true

otymartin commented 6 years ago

@Ariandr ohh beautiful will take a look now.

Thanks

apwelsh commented 6 years ago

@Ariandr your fix does not work. The keyboard is way too high on the screen, and the text view is not bound by the safe area, resulting in messages appearing below the keyboard.

Ariandr commented 6 years ago

@apwelsh Have you installed my fork or just inserted the piece of code? I don't have such problems in my project using the fork. The only problem is when the keyboard is shown, the text view is not placed precisely above it.

Maybe you have a special setup which causes those problems?

screenshot at may 29 12-40-55 screenshot at may 29 12-41-30 screenshot at may 29 12-45-59

DavidFaraday commented 6 years ago

Any idea how to fix this with iOS12? on iOS 11.4 this worked fine

in JSQMessagesInputToolbar.m

-(void) didMoveToWindow{
[super didMoveToWindow];
 if (@available(iOS 11.0, *)) {
     [[self bottomAnchor] constraintLessThanOrEqualToSystemSpacingBelowAnchor:self.window.safeAreaLayoutGuide.bottomAnchor multiplier:1.0].active = YES;
     }
}
apwelsh commented 6 years ago

So, this is what worked for me, I went back to the official pod source for JSQMessagesViewController, and then in my code, I create a new Category JSQMessagesInputToolBar+SafeArea

The header file looks like this:

//
//  JSQMessagesInputToolbar+SafeArea.h
//  PeerRate
//

#import <JSQMessagesViewController/JSQMessagesViewController.h>

@interface JSQMessagesInputToolbar (SafeArea)

- (void)didMoveToWindow;

@end

And the implementation class looks like this

//
//  JSQMessagesInputToolbar+SafeArea.m
//

#import "JSQMessagesInputToolbar+SafeArea.h"

@implementation JSQMessagesInputToolbar (SafeArea)

- (void)didMoveToWindow {
    [super didMoveToWindow];
    NSLog(@"didMoveToWindow");
    if (@available(iOS 11.0, *)) {

        UILayoutGuide * _Nonnull safeArea = self.window.safeAreaLayoutGuide;
        if (safeArea) {
            NSLayoutYAxisAnchor * _Nonnull bottomAnchor = safeArea.bottomAnchor;
            [[self bottomAnchor] constraintLessThanOrEqualToSystemSpacingBelowAnchor:bottomAnchor multiplier:1.0].active = YES;
        }
    }
}

@end

No other changes were required, except to include the JSQMessagesInputToolbar in my ChatViewController.m class that subclasses the JSQMessagesViewController.

sebasdeweert commented 6 years ago

This will work: https://stackoverflow.com/a/47810515/2084596

LatteCat commented 5 years ago

@nbonamy Found a solution without empty space below input toolbar, the idea that not the whole toolbar should be above the safe area, but only its content:

-(void) didMoveToWindow{
    [super didMoveToWindow];
    if (@available(iOS 11.0, *)) {
        if (self.window.safeAreaLayoutGuide != nil) {
            [[self.contentView bottomAnchor] constraintLessThanOrEqualToSystemSpacingBelowAnchor:self.window.safeAreaLayoutGuide.bottomAnchor multiplier:1.0].active = YES;
        }
    }
}

It works! thx

Artem85 commented 5 years ago

So, this is what worked for me, I went back to the official pod source for JSQMessagesViewController, and then in my code, I create a new Category JSQMessagesInputToolBar+SafeArea

The header file looks like this:

//
//  JSQMessagesInputToolbar+SafeArea.h
//  PeerRate
//

#import <JSQMessagesViewController/JSQMessagesViewController.h>

@interface JSQMessagesInputToolbar (SafeArea)

- (void)didMoveToWindow;

@end

And the implementation class looks like this

//
//  JSQMessagesInputToolbar+SafeArea.m
//

#import "JSQMessagesInputToolbar+SafeArea.h"

@implementation JSQMessagesInputToolbar (SafeArea)

- (void)didMoveToWindow {
    [super didMoveToWindow];
    NSLog(@"didMoveToWindow");
    if (@available(iOS 11.0, *)) {

        UILayoutGuide * _Nonnull safeArea = self.window.safeAreaLayoutGuide;
        if (safeArea) {
            NSLayoutYAxisAnchor * _Nonnull bottomAnchor = safeArea.bottomAnchor;
            [[self bottomAnchor] constraintLessThanOrEqualToSystemSpacingBelowAnchor:bottomAnchor multiplier:1.0].active = YES;
        }
    }
}

@end

No other changes were required, except to include the JSQMessagesInputToolbar in my ChatViewController.m class that subclasses the JSQMessagesViewController.

Did you #import "JSQMessagesInputToolbar+SafeArea.m" in your ChatViewController.m class?

PasqualePuzio commented 5 years ago

The solution proposed by @apwelsh worked for me

tecworldit commented 5 years ago

Beautiful, thanks for this @chrisballinger and really good idea opening a consolidated iPhone X issue page. @chrisballinger question how did you solve safe-area guide issue in your own project? You mentioned the old xib being cached, I can't seem to solve it in the demo or my own project.

Update This persons answer is the only solution that worked for me. Placing this code in JSQMessagesInputToolbar.m

-(void) didMoveToWindow{
[super didMoveToWindow];
 if (@available(iOS 11.0, *)) {
     [[self bottomAnchor] constraintLessThanOrEqualToSystemSpacingBelowAnchor:self.window.safeAreaLayoutGuide.bottomAnchor multiplier:1.0].active = YES;
     }
}

https://stackoverflow.com/questions/46439975/jsqmessageviewcontroller-ios11-toolbar

Perfect