[Accessoryview] fix#893
Conversation
|
@JulienKode Nice! So if the width is set to large your just shrinking it? I think that makes sense so that it fits the overall message cell padding. @SD10 what do you think? Sent with GitHawk |
@nathantannar4 it’s supposed to do that as we take care of the size of the accessoryView Sent with GitHawk |
| } | ||
| default: | ||
| origin.y = attributes.cellTopLabelSize.height + attributes.messageTopLabelSize.height + attributes.messageContainerPadding.top | ||
| if attributes.accessoryViewSize.height > attributes.messageContainerSize.height { |
There was a problem hiding this comment.
Can you explain this to me a little bit more? I don't know why it's necessary
There was a problem hiding this comment.
We are doing that here because usually the origin.y of the view is just under the others items
But If the accessoryView is bigger than the messageContentCell,
It will cause the accessory view to render outisde the cell
As the accessory view is at the center of the messageContentCell if the accessoryView is bigger, he will go outside of the cell
So if the accessoryView is bigger, we render the messageContentCell, at the middle of the cell then the accessoryView fit perfectly the cell
There was a problem hiding this comment.
There was a problem hiding this comment.
Hey @JulienKode, thanks for explaining this to me 👍 I think the real issue is in the height calculation for the cell? I'm sure if this PR solves the issue the correct way but I totally understand what you're saying is the problem
There was a problem hiding this comment.
There may actually not be a great way to solve this as I think more about it
There was a problem hiding this comment.
I agree that’s not a good way to handle this new case, what did you suggest ?
May be add an enum somewhere
There was a problem hiding this comment.
@SD10 @JulienKode I think the current state is good, it doesnt break the cell layout as all subviews are rendered within the bounds. I think if we want to develop the reliability for rendering options in this case it could be tabled for a future release.
I think we really need to release 1.1, particularly because of the content inset bug people keep facing and the Swift 4.2 update
|
@JulienKode This does a good job aligning the accessoryView to the center, but we still need to make some changes. Im gonna merge this to my branch and then work on it. TODO:
|
|
Thanks, I'll take a look at this |
With an Accessoryview of 100 x 300
#834