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

Fix JSQAudioMediaItem Outgoing design. #1577

Closed
wants to merge 1 commit into from

Conversation

hassan8357
Copy link

Pull request checklist

  • All tests pass. Demo project builds and runs.
  • I have resolved any merge conflicts.
  • I have followed the coding style, and reviewed the contributing guidelines. Confirmation: ____

This fixes issue #___JSQAudioMediaItem Outgoing design.

What's in this pull request?

Fix JSQAudioMediaItem Outgoing design
Before
After

...

self.audioViewAttributes.playButtonImage.size.height);
CGRect buttonFrame;
if (!self.appliesMediaViewMaskAsOutgoing) {
buttonFrame = CGRectMake(self.audioViewAttributes.controlInsets.left + 5,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't want this hardcoded 5 here.

we should handle this somehow in controlInsets

@jessesquires jessesquires added this to the 7.2.1 milestone May 15, 2016
@jessesquires
Copy link
Owner

Thanks so much @hassan8357 ! 😄

@eliburke -- what's the best way to handle this?

@eliburke
Copy link
Collaborator

@hassan8357 thanks for catching this.

@jessesquires I need to think on the best way to fix, but this isn't the first time I've bumped into alignment issues related to tail vs tailless bubbles. For example, aligning the cell top label with the bubble, rather than the bubble's tail, on an incoming message.

What do you think about adding a CGFloat for the tail width somewhere (perhaps JSQMessagesCollectionViewLayoutAttributes) and then I can start to look for places where we can use it to adjust for situations like this.

@jessesquires
Copy link
Owner

What do you think about adding a CGFloat for the tail width somewhere

I don't think we'll need this for v8.0. Since we'll be section-based, the cell labels can become header/footer views.

@eliburke
Copy link
Collaborator

eliburke commented May 16, 2016

I don't see how header/footer views solve the problem of aligning with the main bubble body. But ignoring them for the moment just consider bubble contents. If i want a square image inside a bubble, or if I want to embed any view or html that does not get masked to the shape of the bubble (like here in the audio media bubble), the width of the tail is an unknown. There is no way to know the left border of the mask.

For now, would it be appropriate to use JSQMessagesCollectionViewLayoutAttributes.textViewFrameInsets.left? (or textViewTextContainerInsets-- I can't remember which does what)

@jessesquires
Copy link
Owner

Oh derp. Sorry, I misread that.

@eliburke
Copy link
Collaborator

Replacement pull request in #1590.

@eliburke eliburke closed this May 23, 2016
@jessesquires
Copy link
Owner

thanks so much for finding and reporting this bug @hassan8357! 👍

@eliburke eliburke mentioned this pull request May 23, 2016
2 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants