Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Messages selection #411

Merged
merged 14 commits into from
Dec 4, 2017
Merged

Messages selection #411

merged 14 commits into from
Dec 4, 2017

Conversation

AntonPalich
Copy link
Contributor

  • Added support for selection indicator to BaseMessagesCollectionViewCell
  • Implemented "Messages Selection" feature in the example app that uses selection indicator from BaseMessagesCollectionViewCell to select/deselect messages in the chat

@AntonPalich AntonPalich requested a review from geegaset November 30, 2017 13:02
@codecov-io
Copy link

codecov-io commented Nov 30, 2017

Codecov Report

Merging #411 into master will decrease coverage by 0.19%.
The diff coverage is 82.96%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #411     +/-   ##
=========================================
- Coverage   65.02%   64.83%   -0.2%     
=========================================
  Files          70       70             
  Lines        4500     4575     +75     
  Branches      232      235      +3     
=========================================
+ Hits         2926     2966     +40     
- Misses       1557     1591     +34     
- Partials       17       18      +1
Impacted Files Coverage Δ
.../Chat Items/BaseMessage/BaseMessageViewModel.swift 86.15% <100%> (-2.46%) ⬇️
...ws/TextMessageCollectionViewCellDefaultStyle.swift 82.65% <100%> (ø) ⬆️
...s/PhotoMessageCollectionViewCellDefaultStyle.swift 78.35% <100%> (ø) ⬆️
...at Items/PhotoMessages/PhotoMessageViewModel.swift 79.31% <100%> (ø) ⬆️
...urce/Chat Items/ChatItemDecorationAttributes.swift 100% <100%> (ø) ⬆️
.../Chat Items/BaseMessage/BaseMessagePresenter.swift 53.27% <8.33%> (-6.02%) ⬇️
...ws/BaseMessageCollectionViewCellDefaultStyle.swift 87.5% <84.21%> (-1.14%) ⬇️
...eMessage/Views/BaseMessageCollectionViewCell.swift 61.14% <86.36%> (+5.29%) ⬆️
...ttoAdditions/Source/Common/UIImage+Additions.swift 30.55% <0%> (-44.45%) ⬇️

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 aaec01d...4b094c2. Read the comment docs.

@@ -101,6 +103,8 @@ open class BaseMessagePresenter<BubbleViewT, ViewModelBuilderT, InteractionHandl
cell.performBatchUpdates({ () -> Void in
self.messageViewModel.showsTail = decorationAttributes.canShowTail
self.messageViewModel.showsAvatar = decorationAttributes.canShowAvatar
self.messageViewModel.isShowingSelectionIndicator = decorationAttributes.isShowingSelectionIndicator
Copy link
Contributor

Choose a reason for hiding this comment

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

the way how you copy all the attributes from one entity to another looks a little bit suspicious. why don't you have the dedicated entity which represents the whole set of properties? in this way copying would look like: self.mesageViewModel.attributes = decorationAttributes.attributes

self.selectionIndicator.image = style.selectionIndicatorIcon(for: self.messageViewModel)
}

private lazy var selectionTapGestureRecognizer: UITapGestureRecognizer = {
Copy link
Contributor

Choose a reason for hiding this comment

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

does it really need to be lazy? it looks like it's used in init.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll fix it

self.bubbleViewFrame = bubbleSize.bma_rect(inContainer: containerRect, xAlignament: .center, yAlignment: .center)
self.failedButtonFrame = failedButtonSize.bma_rect(inContainer: containerRect, xAlignament: .center, yAlignment: .center)
self.avatarViewFrame = avatarSize.bma_rect(inContainer: containerRect, xAlignament: .center, yAlignment: parameters.avatarVerticalAlignment)
self.bubbleViewFrame = bubbleSize.bma_rect(
Copy link
Contributor

Choose a reason for hiding this comment

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

is this correct to use bma_ prefixes in Chatto code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Anton Schukin added 2 commits December 1, 2017 10:27
@AntonPalich AntonPalich merged commit 1d3b591 into master Dec 4, 2017
@AntonPalich AntonPalich deleted the message_selection branch December 4, 2017 10:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants