Skip to content
This repository has been archived by the owner on Jun 7, 2020. It is now read-only.

[NEW] Audio message with message headers #2250

Merged

Conversation

filipealva
Copy link
Collaborator

@filipealva filipealva commented Oct 15, 2018

@RocketChat/ios

I'll split these changes into more PRs to avoid a big one.

@codecov
Copy link

codecov bot commented Oct 15, 2018

Codecov Report

Merging #2250 into feature/integrate-rc-view-controller will decrease coverage by 0.42%.
The diff coverage is 0.57%.

Impacted file tree graph

@@                           Coverage Diff                            @@
##           feature/integrate-rc-view-controller    #2250      +/-   ##
========================================================================
- Coverage                                 26.95%   26.53%   -0.43%     
========================================================================
  Files                                       419      423       +4     
  Lines                                     15746    15877     +131     
========================================================================
- Hits                                       4245     4213      -32     
- Misses                                    11501    11664     +163
Impacted Files Coverage Δ
...ket.Chat/Views/Chat/New Chat/Cells/AudioCell.swift 0% <0%> (ø)
...ews/Chat/New Chat/Cells/BaseAudioMessageCell.swift 0% <0%> (ø)
...t/Views/Chat/New Chat/Cells/BasicMessageCell.swift 0% <0%> (ø) ⬆️
...Controllers/Chat/ChatSections/MessageSection.swift 1.5% <0%> (-0.13%) ⬇️
.../Views/Chat/New Chat/Cells/MessageHeaderCell.swift 0% <0%> (ø)
...Chat/New Chat/ChatItems/AudioMessageChatItem.swift 0% <0%> (ø) ⬆️
...t/Views/Chat/New Chat/Cells/AudioMessageCell.swift 0% <0%> (ø) ⬆️
...hat/New Chat/ChatItems/MessageHeaderChatItem.swift 0% <0%> (ø)
...Chat/Controllers/Chat/MessagesViewController.swift 41.93% <100%> (+0.95%) ⬆️
...anagers/Model/AuthManager/AuthManagerRecover.swift 36.36% <0%> (-60.61%) ⬇️
... and 10 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b0362a9...981cabf. Read the comment docs.

@@ -94,12 +110,12 @@ final class MessageSection: ChatSection {
).wrapped)
}

if !object.isSequential {
if !object.isSequential && !object.message.text.isEmpty {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should you use shouldAppendMessageHeader here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure

@@ -2,15 +2,16 @@
// AudioMessageCell.swift
// Rocket.Chat
//
// Created by Rafael Streit on 28/09/18.
// Created by Filipe Alvarenga on 15/10/18.
Copy link
Contributor

Choose a reason for hiding this comment

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

:trollface:

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hahah, this is because I refactored the former AudioMessageCell to AudioCell then created a new file called AudioMessageCell but the diff couldn't track it right

self.emoji = emoji
self.date = date
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

@rafaelks rafaelks left a comment

Choose a reason for hiding this comment

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

Looking really nice! 👍

@rafaelks rafaelks merged commit 2e96f1f into feature/integrate-rc-view-controller Oct 15, 2018
@rafaelks rafaelks added this to the 3.2.0 milestone Oct 15, 2018
@rafaelks rafaelks deleted the feature/audio-message-headers branch October 15, 2018 14:06
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants