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.

Sign up for GitHub

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

Jump to bottom

Message bubbles: Use layout constants instead magic numbers #5588

Merged
merged 14 commits into from
Feb 16, 2022
Merged

Message bubbles: Use layout constants instead magic numbers #5588

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
ddc632f
BubbleRoomCellLayoutConstants: Update constants and add new ones.
SBiOSoftWhare Feb 15, 2022
ac29be5
Text message bubbles: Use constants.
SBiOSoftWhare Feb 15, 2022
998fe0d
File cells: Use constants.
SBiOSoftWhare Feb 15, 2022
f65941e
Location sharing cells: Use constants.
SBiOSoftWhare Feb 15, 2022
8def241
Poll cells: Use constants.
SBiOSoftWhare Feb 15, 2022
7a31b58
Voice message cells: Use constants.
SBiOSoftWhare Feb 15, 2022
d4cebdf
PollBaseBubbleCell: Use constants.
SBiOSoftWhare Feb 15, 2022
c6808ac
BubbleRoomCellLayoutUpdater: User constants.
SBiOSoftWhare Feb 15, 2022
eede625
BubbleRoomCellLayoutConstants: Add new decoration constraints.
SBiOSoftWhare Feb 15, 2022
0b8f4d9
RoomMessageBubbleBackgroundView: Use constants.
SBiOSoftWhare Feb 15, 2022
9f0dae2
FileWithoutThumbnailCellContentView: Use constants.
SBiOSoftWhare Feb 15, 2022
0936231
BubbleRoomTimelineCellDecorator: Use layout constants.
SBiOSoftWhare Feb 15, 2022
f852133
BubbleRoomCellLayoutUpdater: Fix outgoing message issue.
SBiOSoftWhare Feb 16, 2022
d032333
Add changes
SBiOSoftWhare Feb 16, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Riot/Modules/Room/Views/BubbleCells/RoomBubbleCellLayout.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ import Foundation
@objcMembers
final class RoomBubbleCellLayout: NSObject {

/// Inner content view margins
static let innerContentViewMargins: UIEdgeInsets = UIEdgeInsets(top: 0, left: 57, bottom: 0.0, right: 0)

// Reactions

static let reactionsViewTopMargin: CGFloat = 1.0
Expand Down
33 changes: 31 additions & 2 deletions Riot/Modules/Room/Views/BubbleCells/Styles/Bubble/BubbleRoomCellLayoutConstants.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,38 @@ import Foundation
@objcMembers
final class BubbleRoomCellLayoutConstants: NSObject {

/// Inner content view margins
static let innerContentViewMargins: UIEdgeInsets = UIEdgeInsets(top: 0, left: 0, bottom: 5.0, right: 0)

// Text message bubbles margins from cell content view

static let outgoingBubbleBackgroundMargins: UIEdgeInsets = UIEdgeInsets(top: 0, left: 80, bottom: 0, right: 34)

static let incomingBubbleBackgroundMargins: UIEdgeInsets = UIEdgeInsets(top: 0, left: 0, bottom: 0, right: 80)
static let incomingBubbleBackgroundMargins: UIEdgeInsets = UIEdgeInsets(top: 0, left: 48, bottom: 0, right: 80)

static let bubbleTextViewInsets: UIEdgeInsets = UIEdgeInsets(top: 5, left: 5, bottom: 5, right: 45)

static let bubbleCornerRadius: CGFloat = 12.0

// Voice message

static let voiceMessagePlaybackViewRightMargin: CGFloat = 40

// Polls

static let pollBubbleBackgroundInsets: UIEdgeInsets = UIEdgeInsets(top: 2, left: 10, bottom: 0, right: 10)

// Decoration margins

// Timestamp margins

/// Timestamp margins for sticker cell, margin is relative to the image view
static let stickerTimestampViewMargins = UIEdgeInsets(top: 0, left: 0, bottom: 20, right: -27) // Right margin is not reliable there, if the text size change this one is not working anymore

/// Timestamp margins inside bubble
static let bubbleTimestampViewMargins = UIEdgeInsets(top: 0, left: 0, bottom: 4.0, right: 8.0)

static let threadSummaryViewMargins: UIEdgeInsets = UIEdgeInsets(top: 0, left: 0, bottom: 5, right: 0)
static let reactionsViewMargins = UIEdgeInsets(top: 4, left: 0, bottom: 0, right: 0)

static let threadSummaryViewMargins: UIEdgeInsets = UIEdgeInsets(top: 8.0, left: 0, bottom: 5, right: 0)
}
21 changes: 8 additions & 13 deletions Riot/Modules/Room/Views/BubbleCells/Styles/Bubble/BubbleRoomCellLayoutUpdater.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,11 +152,10 @@ class BubbleRoomCellLayoutUpdater: RoomCellLayoutUpdating {

private func getIncomingMessageTextViewInsets(from bubbleCell: MXKRoomBubbleTableViewCell) -> UIEdgeInsets {

let bubbleBgRightMargin: CGFloat = 45
let messageViewMarginTop: CGFloat = 0
let messageViewMarginBottom: CGFloat = -0
let messageViewMarginBottom: CGFloat = 0
let messageViewMarginLeft: CGFloat = 0
let messageViewMarginRight: CGFloat = 80 + bubbleBgRightMargin
let messageViewMarginRight: CGFloat = BubbleRoomCellLayoutConstants.incomingBubbleBackgroundMargins.right + BubbleRoomCellLayoutConstants.bubbleTextViewInsets.right

let messageViewInsets = UIEdgeInsets(top: messageViewMarginTop, left: messageViewMarginLeft, bottom: messageViewMarginBottom, right: messageViewMarginRight)

Expand All @@ -180,13 +179,12 @@ class BubbleRoomCellLayoutUpdater: RoomCellLayoutUpdating {
}

private func getOutgoingMessageTextViewMargins(from bubbleCell: MXKRoomBubbleTableViewCell) -> UIEdgeInsets {

let innerContentLeftMargin: CGFloat = 57

let messageViewMarginTop: CGFloat = 0
let messageViewMarginBottom: CGFloat = 0
let messageViewMarginLeft: CGFloat = 80.0 + innerContentLeftMargin
let messageViewMarginRight: CGFloat = 78.0
let messageViewMarginLeft =
BubbleRoomCellLayoutConstants.outgoingBubbleBackgroundMargins.left + RoomBubbleCellLayout.innerContentViewMargins.left
Copy link
Member

Choose a reason for hiding this comment

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

Just a double check that this is adding innerContentViewMargins whereas the right margins are adding bubbleTextViewInsets.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch

let messageViewMarginRight = BubbleRoomCellLayoutConstants.outgoingBubbleBackgroundMargins.right + BubbleRoomCellLayoutConstants.bubbleTextViewInsets.right

let messageViewInsets = UIEdgeInsets(top: messageViewMarginTop, left: messageViewMarginLeft, bottom: messageViewMarginBottom, right: messageViewMarginRight)

Expand Down Expand Up @@ -230,10 +228,8 @@ class BubbleRoomCellLayoutUpdater: RoomCellLayoutUpdating {
}

let contentView = cell.contentView

// TODO: Use constants
// Same as URL preview
let rightMargin: CGFloat = 34.0

let rightMargin = BubbleRoomCellLayoutConstants.outgoingBubbleBackgroundMargins.right

if let attachViewLeadingConstraint = cell.attachViewLeadingConstraint {
attachViewLeadingConstraint.isActive = false
Expand Down Expand Up @@ -263,8 +259,7 @@ class BubbleRoomCellLayoutUpdater: RoomCellLayoutUpdating {

let contentView = cell.contentView

// TODO: Use constants
let leftMargin: CGFloat = 67
let leftMargin: CGFloat = BubbleRoomCellLayoutConstants.incomingBubbleBackgroundMargins.left

let leftConstraint = attachmentView.leadingAnchor.constraint(equalTo: contentView.leadingAnchor, constant: -leftMargin)

Expand Down
52 changes: 21 additions & 31 deletions Riot/Modules/Room/Views/BubbleCells/Styles/Bubble/BubbleRoomTimelineCellDecorator.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ class BubbleRoomTimelineCellDecorator: PlainRoomTimelineCellDecorator {
let attachmentView = cell.attachmentView {

// Prevent overlap with send status icon
let bottomMargin: CGFloat = 20.0
let rightMargin: CGFloat = -27.0
let bottomMargin: CGFloat = BubbleRoomCellLayoutConstants.stickerTimestampViewMargins.bottom
let rightMargin: CGFloat = BubbleRoomCellLayoutConstants.stickerTimestampViewMargins.right

self.addTimestampLabel(timestampLabel,
to: cell,
Expand Down Expand Up @@ -105,43 +105,38 @@ class BubbleRoomTimelineCellDecorator: PlainRoomTimelineCellDecorator {
let cellContentView = cell.contentView

cellContentView.addSubview(reactionsView)

// TODO: Use constants
let topMargin: CGFloat = 4.0

let topMargin: CGFloat = RoomBubbleCellLayout.reactionsViewTopMargin
let leftMargin: CGFloat
let rightMargin: CGFloat

// Incoming message
if cellData.isIncoming {

var incomingLeftMargin = RoomBubbleCellLayout.reactionsViewLeftMargin
var incomingLeftMargin = BubbleRoomCellLayoutConstants.incomingBubbleBackgroundMargins.left

if cellData.containsBubbleComponentWithEncryptionBadge {
incomingLeftMargin += RoomBubbleCellLayout.encryptedContentLeftMargin
}

leftMargin = incomingLeftMargin - 6.0
leftMargin = incomingLeftMargin

// TODO: Use constants
let messageViewMarginRight: CGFloat = 42.0
rightMargin = BubbleRoomCellLayoutConstants.incomingBubbleBackgroundMargins.right

rightMargin = messageViewMarginRight
} else {
// Outgoing message

reactionsView.alignment = .right

// TODO: Use constants
var outgoingLeftMargin: CGFloat = 80.0

var outgoingLeftMargin = BubbleRoomCellLayoutConstants.outgoingBubbleBackgroundMargins.left

if cellData.containsBubbleComponentWithEncryptionBadge {
outgoingLeftMargin += RoomBubbleCellLayout.encryptedContentLeftMargin
}

leftMargin = outgoingLeftMargin

// TODO: Use constants
rightMargin = 33

rightMargin = BubbleRoomCellLayoutConstants.outgoingBubbleBackgroundMargins.right
}

let leadingConstraint = reactionsView.leadingAnchor.constraint(equalTo: cellContentView.leadingAnchor, constant: leftMargin)
Expand Down Expand Up @@ -190,14 +185,11 @@ class BubbleRoomTimelineCellDecorator: PlainRoomTimelineCellDecorator {
leftMargin += RoomBubbleCellLayout.encryptedContentLeftMargin
}

leftMargin-=5.0

leadingOrTrailingConstraint = urlPreviewView.leadingAnchor.constraint(equalTo: cellContentView.leadingAnchor, constant: leftMargin)
} else {
// Outgoing message

// TODO: Use constants
let rightMargin: CGFloat = 34.0
let rightMargin: CGFloat = BubbleRoomCellLayoutConstants.outgoingBubbleBackgroundMargins.right

leadingOrTrailingConstraint = urlPreviewView.trailingAnchor.constraint(equalTo: cellContentView.trailingAnchor, constant: -rightMargin)
}
Expand Down Expand Up @@ -238,33 +230,31 @@ class BubbleRoomTimelineCellDecorator: PlainRoomTimelineCellDecorator {
// Incoming message
if cellData.isIncoming {

leftMargin = RoomBubbleCellLayout.reactionsViewLeftMargin
leftMargin = BubbleRoomCellLayoutConstants.incomingBubbleBackgroundMargins.left
if cellData.containsBubbleComponentWithEncryptionBadge {
leftMargin += RoomBubbleCellLayout.encryptedContentLeftMargin
}

leftMargin-=5.0

rightMargin = RoomBubbleCellLayout.reactionsViewRightMargin
rightMargin = BubbleRoomCellLayoutConstants.incomingBubbleBackgroundMargins.right

leadingConstraint = threadSummaryView.leadingAnchor.constraint(equalTo: cellContentView.leadingAnchor,
constant: leftMargin)
trailingConstraint = threadSummaryView.trailingAnchor.constraint(lessThanOrEqualTo: cellContentView.trailingAnchor,
constant: -rightMargin)
} else {
// Outgoing message

// TODO: Use constants
leftMargin = 80.0
rightMargin = 34.0

leftMargin = BubbleRoomCellLayoutConstants.outgoingBubbleBackgroundMargins.left
rightMargin = BubbleRoomCellLayoutConstants.outgoingBubbleBackgroundMargins.right

leadingConstraint = threadSummaryView.leadingAnchor.constraint(greaterThanOrEqualTo: cellContentView.leadingAnchor,
constant: leftMargin)
trailingConstraint = threadSummaryView.trailingAnchor.constraint(equalTo: cellContentView.trailingAnchor,
constant: -rightMargin)
}

let topMargin = RoomBubbleCellLayout.threadSummaryViewTopMargin + 15.0
let topMargin = RoomBubbleCellLayout.threadSummaryViewTopMargin

let height = ThreadSummaryView.contentViewHeight(forThread: threadSummaryView.thread,
fitting: cellData.maxTextViewWidth)

Expand Down Expand Up @@ -376,8 +366,8 @@ class BubbleRoomTimelineCellDecorator: PlainRoomTimelineCellDecorator {
to cell: MXKRoomBubbleTableViewCell,
on containerView: UIView,
constrainingView: UIView,
rightMargin: CGFloat = 8.0,
bottomMargin: CGFloat = 4.0) {
rightMargin: CGFloat = BubbleRoomCellLayoutConstants.bubbleTimestampViewMargins.right,
bottomMargin: CGFloat = BubbleRoomCellLayoutConstants.bubbleTimestampViewMargins.bottom) {
timestampLabel.translatesAutoresizingMaskIntoConstraints = false

cell.addTemporarySubview(timestampLabel)
Expand Down
9 changes: 1 addition & 8 deletions ...Styles/Bubble/Cells/FileWithoutThumbnail/Common/FileWithoutThumbnailCellContentView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,6 @@ import Reusable

final class FileWithoutThumbnailCellContentView: UIView, NibLoadable {

// MARK: - Constants

private enum Constants {
// TODO: Reuse constants, same as bubble bg
static let cornerRadius: CGFloat = 12.0
}

// MARK: - Properties

// MARK: Outlets
Expand Down Expand Up @@ -61,7 +54,7 @@ final class FileWithoutThumbnailCellContentView: UIView, NibLoadable {
override func layoutSubviews() {
super.layoutSubviews()

self.layer.cornerRadius = Constants.cornerRadius
self.layer.cornerRadius = BubbleRoomCellLayoutConstants.bubbleCornerRadius
}

// MARK: - Public
Expand Down
8 changes: 2 additions & 6 deletions ...s/Bubble/Cells/FileWithoutThumbnail/Incoming/FileWithoutThumbnailIncomingBubbleCell.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,8 @@ class FileWithoutThumbnailIncomingBubbleCell: FileWithoutThumbnailBaseBubbleCell

bubbleCellContentView?.showSenderInfo = true

// TODO: Use constants
let messageViewMarginRight: CGFloat = 80
let messageLeftMargin: CGFloat = 48

bubbleCellContentView?.innerContentViewTrailingConstraint.constant = messageViewMarginRight
bubbleCellContentView?.innerContentViewLeadingConstraint.constant = messageLeftMargin
bubbleCellContentView?.innerContentViewLeadingConstraint.constant = BubbleRoomCellLayoutConstants.incomingBubbleBackgroundMargins.left
bubbleCellContentView?.innerContentViewTrailingConstraint.constant = BubbleRoomCellLayoutConstants.incomingBubbleBackgroundMargins.right

self.setupBubbleDecorations()
}
Expand Down
9 changes: 2 additions & 7 deletions ...ileWithoutThumbnail/Outgoing/FileWithoutThumbnailOutoingWithoutSenderInfoBubbleCell.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,8 @@ class FileWithoutThumbnailOutoingWithoutSenderInfoBubbleCell: FileWithoutThumbna

bubbleCellContentView?.showSenderInfo = false

// TODO: Use constants
// Same as outgoing message
let rightMargin: CGFloat = 34.0
let leftMargin: CGFloat = 80.0

bubbleCellContentView?.innerContentViewTrailingConstraint.constant = rightMargin
bubbleCellContentView?.innerContentViewLeadingConstraint.constant = leftMargin
bubbleCellContentView?.innerContentViewLeadingConstraint.constant = BubbleRoomCellLayoutConstants.outgoingBubbleBackgroundMargins.left
bubbleCellContentView?.innerContentViewTrailingConstraint.constant = BubbleRoomCellLayoutConstants.outgoingBubbleBackgroundMargins.right

self.setupBubbleDecorations()
}
Expand Down
8 changes: 2 additions & 6 deletions .../Views/BubbleCells/Styles/Bubble/Cells/Location/Incoming/LocationIncomingBubbleCell.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,8 @@ class LocationIncomingBubbleCell: LocationBubbleCell, BubbleIncomingRoomCellProt
override func setupViews() {
super.setupViews()

// TODO: Use constants
let messageViewMarginRight: CGFloat = 80
let messageLeftMargin: CGFloat = 48

bubbleCellContentView?.innerContentViewTrailingConstraint.constant = messageViewMarginRight
bubbleCellContentView?.innerContentViewLeadingConstraint.constant = messageLeftMargin
bubbleCellContentView?.innerContentViewLeadingConstraint.constant = BubbleRoomCellLayoutConstants.incomingBubbleBackgroundMargins.left
bubbleCellContentView?.innerContentViewTrailingConstraint.constant = BubbleRoomCellLayoutConstants.incomingBubbleBackgroundMargins.right

self.setupBubbleDecorations()
}
Expand Down
3 changes: 1 addition & 2 deletions ...s/Styles/Bubble/Cells/Location/Outgoing/LocationOutgoingWithoutSenderInfoBubbleCell.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,8 @@ class LocationOutgoingWithoutSenderInfoBubbleCell: LocationBubbleCell, BubbleOut

bubbleCellContentView?.showSenderInfo = false

bubbleCellContentView?.innerContentViewTrailingConstraint.constant = BubbleRoomCellLayoutConstants.outgoingBubbleBackgroundMargins.right

bubbleCellContentView?.innerContentViewLeadingConstraint.constant = BubbleRoomCellLayoutConstants.outgoingBubbleBackgroundMargins.left
bubbleCellContentView?.innerContentViewTrailingConstraint.constant = BubbleRoomCellLayoutConstants.outgoingBubbleBackgroundMargins.right

self.setupBubbleDecorations()
}
Expand Down
10 changes: 4 additions & 6 deletions ...les/Room/Views/BubbleCells/Styles/Bubble/Cells/Poll/Incoming/PollIncomingBubbleCell.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,11 @@ class PollIncomingBubbleCell: PollBaseBubbleCell, BubbleIncomingRoomCellProtocol
override func setupViews() {
super.setupViews()

// TODO: Use constants
let bubbleBackgroundSideMargin: CGFloat = 10
let messageViewMarginRight: CGFloat = 80 + bubbleBackgroundSideMargin
let messageLeftMargin: CGFloat = 48 + bubbleBackgroundSideMargin
let leftMargin: CGFloat = BubbleRoomCellLayoutConstants.incomingBubbleBackgroundMargins.left + BubbleRoomCellLayoutConstants.pollBubbleBackgroundInsets.left
let rightMargin: CGFloat = BubbleRoomCellLayoutConstants.incomingBubbleBackgroundMargins.right + BubbleRoomCellLayoutConstants.pollBubbleBackgroundInsets.right

bubbleCellContentView?.innerContentViewTrailingConstraint.constant = messageViewMarginRight
bubbleCellContentView?.innerContentViewLeadingConstraint.constant = messageLeftMargin
bubbleCellContentView?.innerContentViewLeadingConstraint.constant = leftMargin
bubbleCellContentView?.innerContentViewTrailingConstraint.constant = rightMargin

self.setupBubbleDecorations()
}
Expand Down
9 changes: 3 additions & 6 deletions ...bbleCells/Styles/Bubble/Cells/Poll/Outgoing/PollOutgoingWithoutSenderInfoBubbleCell.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,9 @@ class PollOutgoingWithoutSenderInfoBubbleCell: PollBaseBubbleCell, BubbleOutgoin
super.setupViews()

bubbleCellContentView?.showSenderInfo = false

// TODO: Use constants
// Same as outgoing message
let bubbleBackgroundSideMargin: CGFloat = 10
let rightMargin: CGFloat = 34.0 + bubbleBackgroundSideMargin
let leftMargin: CGFloat = 80.0 + bubbleBackgroundSideMargin

let leftMargin: CGFloat = BubbleRoomCellLayoutConstants.outgoingBubbleBackgroundMargins.left + BubbleRoomCellLayoutConstants.pollBubbleBackgroundInsets.left
let rightMargin: CGFloat = BubbleRoomCellLayoutConstants.outgoingBubbleBackgroundMargins.right + BubbleRoomCellLayoutConstants.pollBubbleBackgroundInsets.right

bubbleCellContentView?.innerContentViewTrailingConstraint.constant = rightMargin
bubbleCellContentView?.innerContentViewLeadingConstraint.constant = leftMargin
Expand Down
8 changes: 4 additions & 4 deletions Riot/Modules/Room/Views/BubbleCells/Styles/Bubble/Cells/Poll/PollBaseBubbleCell.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,10 @@ class PollBaseBubbleCell: PollBubbleCell {
private func addBubbleBackgroundView(_ bubbleBackgroundView: RoomMessageBubbleBackgroundView,
to pollView: UIView) {

let topMargin: CGFloat = 2.0
let leftMargin: CGFloat = 10.0
let rightMargin: CGFloat = 10.0 // Add extra space for timestamp
let bottomMargin: CGFloat = 0.0 //
let topMargin = BubbleRoomCellLayoutConstants.pollBubbleBackgroundInsets.top
let leftMargin = BubbleRoomCellLayoutConstants.pollBubbleBackgroundInsets.left
let rightMargin = BubbleRoomCellLayoutConstants.pollBubbleBackgroundInsets.right
let bottomMargin = BubbleRoomCellLayoutConstants.pollBubbleBackgroundInsets.bottom

let topAnchor = pollView.topAnchor
let leadingAnchor = pollView.leadingAnchor
Expand Down
Loading