Overlapping content

[gingerbeardman]

Not sure how this happened. I was trying to like the comment and it didn't work, so I then tapped the content to expand it and the emoji thing stayed where it was.

Bug Report Dump (Auto-generated)

Version 1.17.0 (1515261323)
Device: iPhone 6s (iOS 11.2.2)
TestFlight: true

[Sherlouk]

I know I definitely raised an issue for this in the past, but can't remember how I worded it and whether it was closed as a can't reproduce or left open for investigation 😬

Are you able to reproduce this at all?

[Sherlouk]

#1013 was the issue I raised, pretty much the same but didn't have the menu open (not sure if that changes anything)

@rnystrom Maybe worth another look since others have experienced now?

[rnystrom]

Can’t do much without a repro atm. If you can use FLEX and dump the view hierarchy that’d give us a start for sure.

_{Sent with GitHawk}

[gingerbeardman]

Yes I can reproduce, the first thing I tried:

1. Open an issue with a collapsed comment
2. Tap the emoji button then quickly
3. Tap the comment above it to expand

I've got a video of me doing it multiple times, but I can't upload it just yet. So here's another image.

[BasThomas]

Can you dump the view hierarchy? You can open FLEX by tapping with three fingers.

[gingerbeardman]

Sure, i have opened Flex but what do I do next?

[Sherlouk]

I too can reproduce it super easily using that exact comment shown above, but struggling with others - So that makes me think it's something to do with the content or the total size of the comment (it's a relatively large text only comment!)

[gingerbeardman]

Here's one from me

[Sherlouk]

Apologies for this, just wanting to see if a lot of basic text will cause this to happen!

Edit// Super doable on this comment, so narrows it down as it's not caused by alternative content types (e.g. images)

Lorem ipsum dolor sit amet, consectetur adipiscing elit. Maecenas erat ipsum, facilisis nec mauris ac, suscipit eleifend orci. Donec aliquam sagittis mollis. Vestibulum ante ipsum primis in faucibus orci luctus et ultrices posuere cubilia Curae;

Ut cursus neque nisl, eu pulvinar urna convallis ut. Fusce in mi nisl. Curabitur sit amet ullamcorper mi. Nulla blandit convallis nisi, ut finibus arcu egestas at. Donec finibus metus eget dui aliquet, quis efficitur risus viverra.

Quisque nec ultricies sem. Nullam sagittis ligula sed lectus pellentesque, sed sollicitudin urna feugiat. Suspendisse euismod sapien non neque porttitor, varius tincidunt felis lobortis. Morbi dictum malesuada nibh eu gravida.

And another

[Sherlouk]

Okay a lot of experimenting and editing the previous comment, I think I've narrowed it down to if you extend the comment and it's new height exceeds out of the screen bounds (plus a buffer).

In the same way a table view cell loads in a little before it comes on screen, I suspect if the new height pushes the reaction cell out of that buffer, it doesn't know where to place it? and so doesn't do anything?

@rnystrom

With the comment above, it's doable with it's current text but removing one line makes it stop. However, adding a new comment and placing the comment right at the top of my screen and then doing it - it doesn't do it either because it has enough space to actually load the reaction cell?

Hope this makes sense, and I could be completely wrong but seems to make sense

[rnystrom]

Everyone give the latest beta a try. Seems like it’s fixed?

_{Sent with GitHawk}

[gingerbeardman]

Fixed for me. Nice!

[Sherlouk]

Can still reproduce really easily following the same steps on the original comment

[gingerbeardman]

Hmm, that'll teach me to test and close a bug at 1AM. Force quitting the app and I can now reproduce.

It could be my hazy memory of last night but when it worked I seem to remember the "fade" being to-grey rather than the to-white it is after relaunching the app?

Of course, I should have taken a screenshot.

[Sherlouk]

So I sorta fixed it by dropping an invalidate layout in here: https://github.com/rnystrom/GitHawk/blob/master/Classes/Issues/Comments/IssueCommentSectionController.swift#L137

collectionContext?.invalidateLayout(for: self, completion: nil)

It doesn't move the menu, but the actual cell does move
🤔

[rnystrom]

Aha! I see now. Ok. I wonder if we just need to do something where if the reaction cell is the first responder, we don't uncollapse? I'll give it a try, brb.

edit: nope that doesn't help. hmm. I'm not familiar w/ how the menu works internally, will need to play around.

[Sherlouk]

Can still reproduce on latest TestFlight 😖 this time the steps are basically to the tap the bubble then tap something else that has an action (either the reaction cell, or I had it work on an image but that's more difficult to reproduce)

_{Sent with GitHawk}

[gingerbeardman]

I can no longer reproduce

_{Sent with GitHawk}

[Sherlouk]

@gingerbeardman Even with the new steps in my previous comment? (Tapping the bubble then the reaction cell?)

I can reproduce quite easily

[gingerbeardman]

Ah, OK, new steps.

Yes, I can reproduce.

[gingerbeardman]

My steps are now:

1. Open the emoji bar
2. Tap an emoji to add it to the comment
3. Tap the comment to expand

[rnystrom]

@Sherlouk @gingerbeardman im gonna let you guys dig and find a solution here. Seems pretty edge case and you have to try hard to repro, so not blocking 1.17.

_{Sent with GitHawk}

[Sherlouk]

@rnystrom Invalidating the layout as I listed above did seem to always force the reaction bar back down to where it should be with the menu being the issue. The menu is no longer an issue, so it should be a final solution -- Not sure if it has implications on performance or anything?

Your opinion?

Happy to leave this if you think it's super low pri, but I've experienced it a couple times without trying what so ever (mostly when there's an image near the fade boundary)

[gingerbeardman]

I don't have to try hard at all to reproduce. First time every time with these steps:

1. Open the emoji bar
2. Tap an emoji to add it to the comment
3. Tap the comment to expand

Here's a video: http://www.youtube.com/watch?v=JJvny6hs0f8

But I agree it shouldn't be a blocker, low priority.

[rnystrom]

@Sherlouk ya if you wanna put up a PR invalidating there we can give that a shot

_{Sent with GitHawk}

[gingerbeardman]

Another one