Skip to content

Commit

Permalink
pr comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@Velin92
Velin92 committed Jul 25, 2024
1 parent 7f45853 commit dc40f0d
Show file tree
Hide file tree
Showing 12 changed files with 72 additions and 76 deletions.
10 changes: 5 additions & 5 deletions ElementX/Resources/Localizations/en.lproj/Localizable.strings
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@
"action_ok" = "OK";
"action_open_settings" = "Settings";
"action_open_with" = "Open with";
"action_pin" = "Pin";
"action_quick_reply" = "Quick reply";
"action_quote" = "Quote";
"action_react" = "React";
Expand Down Expand Up @@ -99,11 +100,10 @@
"action_take_photo" = "Take photo";
"action_tap_for_options" = "Tap for options";
"action_try_again" = "Try again";
"action_unpin" = "Unpin";
"action_view_source" = "View source";
"action_yes" = "Yes";
"action.load_more" = "Load more";
"action.pin" = "Pin";
"action.unpin" = "Unpin";
"common_about" = "About";
"common_acceptable_use_policy" = "Acceptable use policy";
"common_advanced_settings" = "Advanced settings";
Expand Down Expand Up @@ -314,9 +314,9 @@
"screen_advanced_settings_element_call_base_url_description" = "Set a custom base URL for Element Call.";
"screen_advanced_settings_element_call_base_url_validation_error" = "Invalid URL, please make sure you include the protocol (http/https) and the correct address.";
"screen_room_mentions_at_room_subtitle" = "Notify the whole room";
"screen.room.pinned_banner_indicator" = "%1$@ of %2$@";
"screen.room.pinned_banner_indicator_description" = "%1$@ Pinned messages";
"screen.room.pinned_banner_view_all_button_title" = "View All";
"screen_room_pinned_banner_indicator" = "%1$@ of %2$@";
"screen_room_pinned_banner_indicator_description" = "%1$@ Pinned messages";
"screen_room_pinned_banner_view_all_button_title" = "View All";
"screen_account_provider_change" = "Change account provider";
"screen_account_provider_form_hint" = "Homeserver address";
"screen_account_provider_form_notice" = "Enter a search term or a domain address.";
Expand Down
33 changes: 14 additions & 19 deletions ElementX/Sources/Generated/Strings.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,8 @@ internal enum L10n {
internal static var actionOpenSettings: String { return L10n.tr("Localizable", "action_open_settings") }
/// Open with
internal static var actionOpenWith: String { return L10n.tr("Localizable", "action_open_with") }
/// Pin
internal static var actionPin: String { return L10n.tr("Localizable", "action_pin") }
/// Quick reply
internal static var actionQuickReply: String { return L10n.tr("Localizable", "action_quick_reply") }
/// Quote
Expand Down Expand Up @@ -232,6 +234,8 @@ internal enum L10n {
internal static var actionTapForOptions: String { return L10n.tr("Localizable", "action_tap_for_options") }
/// Try again
internal static var actionTryAgain: String { return L10n.tr("Localizable", "action_try_again") }
/// Unpin
internal static var actionUnpin: String { return L10n.tr("Localizable", "action_unpin") }
/// View source
internal static var actionViewSource: String { return L10n.tr("Localizable", "action_view_source") }
/// Yes
Expand Down Expand Up @@ -1629,6 +1633,16 @@ internal enum L10n {
internal static var screenRoomNotificationSettingsModeMentionsAndKeywords: String { return L10n.tr("Localizable", "screen_room_notification_settings_mode_mentions_and_keywords") }
/// In this room, notify me for
internal static var screenRoomNotificationSettingsRoomCustomSettingsTitle: String { return L10n.tr("Localizable", "screen_room_notification_settings_room_custom_settings_title") }
/// %1$@ of %2$@
internal static func screenRoomPinnedBannerIndicator(_ p1: Any, _ p2: Any) -> String {
return L10n.tr("Localizable", "screen_room_pinned_banner_indicator", String(describing: p1), String(describing: p2))
}
/// %1$@ Pinned messages
internal static func screenRoomPinnedBannerIndicatorDescription(_ p1: Any) -> String {
return L10n.tr("Localizable", "screen_room_pinned_banner_indicator_description", String(describing: p1))
}
/// View All
internal static var screenRoomPinnedBannerViewAllButtonTitle: String { return L10n.tr("Localizable", "screen_room_pinned_banner_view_all_button_title") }
/// Send again
internal static var screenRoomRetrySendMenuSendAgainAction: String { return L10n.tr("Localizable", "screen_room_retry_send_menu_send_again_action") }
/// Your message failed to send
Expand Down Expand Up @@ -2229,10 +2243,6 @@ internal enum L10n {
internal enum Action {
/// Load more
internal static var loadMore: String { return L10n.tr("Localizable", "action.load_more") }
/// Pin
internal static var pin: String { return L10n.tr("Localizable", "action.pin") }
/// Unpin
internal static var unpin: String { return L10n.tr("Localizable", "action.unpin") }
}

internal enum Common {
Expand All @@ -2243,21 +2253,6 @@ internal enum L10n {
/// Send to
internal static var sendTo: String { return L10n.tr("Localizable", "common.send_to") }
}

internal enum Screen {
internal enum Room {
/// %1$@ of %2$@
internal static func pinnedBannerIndicator(_ p1: Any, _ p2: Any) -> String {
return L10n.tr("Localizable", "screen.room.pinned_banner_indicator", String(describing: p1), String(describing: p2))
}
/// %1$@ Pinned messages
internal static func pinnedBannerIndicatorDescription(_ p1: Any) -> String {
return L10n.tr("Localizable", "screen.room.pinned_banner_indicator_description", String(describing: p1))
}
/// View All
internal static var pinnedBannerViewAllButtonTitle: String { return L10n.tr("Localizable", "screen.room.pinned_banner_view_all_button_title") }
}
}
}
// swiftlint:enable explicit_type_interface function_parameter_count identifier_name line_length
// swiftlint:enable nesting type_body_length type_name vertical_whitespace_opening_braces
Expand Down
20 changes: 10 additions & 10 deletions ElementX/Sources/Mocks/Generated/GeneratedMocks.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8251,23 +8251,23 @@ class RoomProxyMock: RoomProxyProtocol {
}
var underlyingIsFavourite: Bool!
var isFavouriteClosure: (() async -> Bool)?
var pinnedEventsCallsCount = 0
var pinnedEventsCalled: Bool {
return pinnedEventsCallsCount > 0
var pinnedEventIDsCallsCount = 0
var pinnedEventIDsCalled: Bool {
return pinnedEventIDsCallsCount > 0
}

var pinnedEvents: [String] {
var pinnedEventIDs: [String] {
get async {
pinnedEventsCallsCount += 1
if let pinnedEventsClosure = pinnedEventsClosure {
return await pinnedEventsClosure()
pinnedEventIDsCallsCount += 1
if let pinnedEventIDsClosure = pinnedEventIDsClosure {
return await pinnedEventIDsClosure()
} else {
return underlyingPinnedEvents
return underlyingPinnedEventIDs
}
}
}
var underlyingPinnedEvents: [String]!
var pinnedEventsClosure: (() async -> [String])?
var underlyingPinnedEventIDs: [String]!
var pinnedEventIDsClosure: (() async -> [String])?
var membership: Membership {
get { return underlyingMembership }
set(value) { underlyingMembership = value }
Expand Down
31 changes: 16 additions & 15 deletions ElementX/Sources/Screens/RoomScreen/RoomScreenModels.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ struct RoomScreenViewState: BindableState {
var pinnedEventsState = PinnedEventsState()

var shouldShowPinBanner: Bool {
isPinningEnabled && !pinnedEventsState.pinnedEvents.isEmpty && lastScrollDirection != .top
isPinningEnabled && !pinnedEventsState.pinnedEventIDs.isEmpty && lastScrollDirection != .top
}

var canJoinCall = false
Expand Down Expand Up @@ -297,37 +297,38 @@ enum ScrollDirection: Equatable {

struct PinnedEventsState: Equatable {
// For now these will only contain and show the event IDs, but in the future they will also contain the content
var pinnedEvents: OrderedSet<String> = [] {
var pinnedEventIDs: OrderedSet<String> = [] {
didSet {
if selectedPin == nil, !pinnedEvents.isEmpty {
selectedPin = pinnedEvents.first
} else if pinnedEvents.isEmpty {
selectedPin = nil
} else if let selectedPin, !pinnedEvents.contains(selectedPin) {
self.selectedPin = pinnedEvents.first
if selectedPinEventID == nil, !pinnedEventIDs.isEmpty {
selectedPinEventID = pinnedEventIDs.first
} else if pinnedEventIDs.isEmpty {
selectedPinEventID = nil
} else if let selectedPinEventID, !pinnedEventIDs.contains(selectedPinEventID) {
self.selectedPinEventID = pinnedEventIDs.first
}
}
}

var selectedPin: String?
var selectedPinEventID: String?

var selectedPinIndex: Int {
guard let selectedPin else {
guard let selectedPinEventID else {
return 0
}
return pinnedEvents.firstIndex(of: selectedPin) ?? 0
return pinnedEventIDs.firstIndex(of: selectedPinEventID) ?? 0
}

// For now we show the event ID as the content, but is just until we have a way to get the real content
var selectedPinContent: AttributedString {
.init(selectedPin ?? "")
.init(selectedPinEventID ?? "")
}

mutating func nextPin() {
guard !pinnedEvents.isEmpty else {
guard !pinnedEventIDs.isEmpty else {
return
}
let currentIndex = selectedPinIndex
let nextIndex = (currentIndex + 1) % pinnedEvents.count
selectedPin = pinnedEvents[nextIndex]
let nextIndex = (currentIndex + 1) % pinnedEventIDs.count
selectedPinEventID = pinnedEventIDs[nextIndex]
}
}
14 changes: 10 additions & 4 deletions ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol
case let .hasScrolled(direction):
state.lastScrollDirection = direction
case .tappedPinBanner:
if let eventID = state.pinnedEventsState.selectedPin {
if let eventID = state.pinnedEventsState.selectedPinEventID {
Task { await focusOnEvent(eventID: eventID) }
}
state.pinnedEventsState.nextPin()
Expand Down Expand Up @@ -404,19 +404,25 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol
}
.store(in: &cancellables)

roomProxy
let roomInfoSubscription = roomProxy
.actionsPublisher
.filter { $0 == .roomInfoUpdate }
.receive(on: DispatchQueue.main)
roomInfoSubscription
.throttle(for: .seconds(1), scheduler: DispatchQueue.main, latest: true)
.sink { [weak self] _ in
guard let self else { return }
state.roomTitle = roomProxy.roomTitle
state.roomAvatar = roomProxy.avatar
state.hasOngoingCall = roomProxy.hasOngoingCall
}
.store(in: &cancellables)
roomInfoSubscription
.receive(on: DispatchQueue.main)
.sink { [weak self] _ in
Task { [weak self] in
guard let self else { return }
// TODO: For now we are using the order coming from the room info but we should instead have a ordered timeline of these events
await state.pinnedEventsState.pinnedEvents = .init(roomProxy.pinnedEvents)
await state.pinnedEventsState.pinnedEventIDs = .init(roomProxy.pinnedEventIDs)
}
}
.store(in: &cancellables)
Expand Down
4 changes: 2 additions & 2 deletions ElementX/Sources/Screens/RoomScreen/View/ItemMenu/TimelineItemMenuAction.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,9 +136,9 @@ enum TimelineItemMenuAction: Identifiable, Hashable {
case .endPoll:
Label(L10n.actionEndPoll, icon: \.pollsEnd)
case .pin:
Label(L10n.Action.pin, icon: \.pin)
Label(L10n.actionPin, icon: \.pin)
case .unpin:
Label(L10n.Action.unpin, icon: \.unpin)
Label(L10n.actionUnpin, icon: \.unpin)
}
}
}
4 changes: 2 additions & 2 deletions ElementX/Sources/Screens/RoomScreen/View/ItemMenu/TimelineItemMenuActionProvider.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ struct TimelineItemMenuActionProvider {
let canCurrentUserRedactSelf: Bool
let canCurrentUserRedactOthers: Bool
let canCurrentUserPin: Bool
let pinnedEvents: Set<String>
let pinnedEventIDs: Set<String>
let isDM: Bool
let isViewSourceEnabled: Bool

Expand Down Expand Up @@ -68,7 +68,7 @@ struct TimelineItemMenuActionProvider {
}

if canCurrentUserPin, let eventID = item.id.eventID {
actions.append(pinnedEvents.contains(eventID) ? .unpin : .pin)
actions.append(pinnedEventIDs.contains(eventID) ? .unpin : .pin)
}

if item.isEditable {
Expand Down
20 changes: 8 additions & 12 deletions ElementX/Sources/Screens/RoomScreen/View/PinnedItemsBanner/PinnedItemsBannerView.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,16 @@ import Compound
import SwiftUI

struct PinnedItemsBannerView: View {
let pinIndex: Int
let pinsCount: Int
let pinContent: AttributedString
let pinnedEventsState: PinnedEventsState

let onMainButtonTap: () -> Void
let onViewAllButtonTap: () -> Void

private var bannerIndicatorDescription: AttributedString {
let index = pinIndex + 1
let index = pinnedEventsState.selectedPinIndex + 1
let boldPlaceholder = "{bold}"
var finalString = AttributedString(L10n.Screen.Room.pinnedBannerIndicatorDescription(boldPlaceholder))
var boldString = AttributedString(L10n.Screen.Room.pinnedBannerIndicator(index, pinsCount))
var finalString = AttributedString(L10n.screenRoomPinnedBannerIndicatorDescription(boldPlaceholder))
var boldString = AttributedString(L10n.screenRoomPinnedBannerIndicator(index, pinnedEventsState.pinnedEventIDs.count))
boldString.bold()
finalString.replace(boldPlaceholder, with: boldString)
return finalString
Expand All @@ -50,7 +48,7 @@ struct PinnedItemsBannerView: View {
Button { onMainButtonTap() } label: {
HStack(spacing: 0) {
HStack(spacing: 10) {
PinnedItemsIndicatorView(pinIndex: pinIndex, pinsCount: pinsCount)
PinnedItemsIndicatorView(pinIndex: pinnedEventsState.selectedPinIndex, pinsCount: pinnedEventsState.pinnedEventIDs.count)
.accessibilityHidden(true)
CompoundIcon(\.pinSolid, size: .small, relativeTo: .compound.bodyMD)
.foregroundColor(Color.compound.iconSecondaryAlpha)
Expand All @@ -65,7 +63,7 @@ struct PinnedItemsBannerView: View {

private var viewAllButton: some View {
Button { onViewAllButtonTap() } label: {
Text(L10n.Screen.Room.pinnedBannerViewAllButtonTitle)
Text(L10n.screenRoomPinnedBannerViewAllButtonTitle)
.font(.compound.bodyMDSemibold)
.foregroundStyle(Color.compound.textPrimary)
.padding(.horizontal, 16)
Expand All @@ -79,7 +77,7 @@ struct PinnedItemsBannerView: View {
.font(.compound.bodySM)
.foregroundColor(.compound.textActionAccent)
.lineLimit(1)
Text(pinContent)
Text(pinnedEventsState.selectedPinContent)
.font(.compound.bodyMD)
.foregroundColor(.compound.textPrimary)
.lineLimit(1)
Expand All @@ -89,9 +87,7 @@ struct PinnedItemsBannerView: View {

struct PinnedItemsBannerView_Previews: PreviewProvider, TestablePreview {
static var previews: some View {
PinnedItemsBannerView(pinIndex: 0,
pinsCount: 3,
pinContent: .init(stringLiteral: "Content"),
PinnedItemsBannerView(pinnedEventsState: .init(pinnedEventIDs: ["Content", "NotShown1", "NotShown2"], selectedPinEventID: "Content"),
onMainButtonTap: { },
onViewAllButtonTap: { })
}
Expand Down
6 changes: 2 additions & 4 deletions ElementX/Sources/Screens/RoomScreen/View/RoomScreen.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ struct RoomScreen: View {
canCurrentUserRedactSelf: context.viewState.canCurrentUserRedactSelf,
canCurrentUserRedactOthers: context.viewState.canCurrentUserRedactOthers,
canCurrentUserPin: context.viewState.canCurrentUserPin,
pinnedEvents: context.viewState.pinnedEventsState.pinnedEvents.set,
pinnedEventIDs: context.viewState.pinnedEventsState.pinnedEventIDs.set,
isDM: context.viewState.isEncryptedOneToOneRoom,
isViewSourceEnabled: context.viewState.isViewSourceEnabled).makeActions()
if let actions {
Expand Down Expand Up @@ -110,9 +110,7 @@ struct RoomScreen: View {
}

private var pinnedItemsBanner: some View {
PinnedItemsBannerView(pinIndex: context.viewState.pinnedEventsState.selectedPinIndex,
pinsCount: context.viewState.pinnedEventsState.pinnedEvents.count,
pinContent: context.viewState.pinnedEventsState.selectedPinContent,
PinnedItemsBannerView(pinnedEventsState: context.viewState.pinnedEventsState,
onMainButtonTap: { context.send(viewAction: .tappedPinBanner) },
onViewAllButtonTap: { context.send(viewAction: .viewAllPins) })
.transition(.move(edge: .top))
Expand Down
2 changes: 1 addition & 1 deletion ElementX/Sources/Screens/RoomScreen/View/Style/TimelineItemBubbledStylerView.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ struct TimelineItemBubbledStylerView<Content: View>: View {
canCurrentUserRedactSelf: context.viewState.canCurrentUserRedactSelf,
canCurrentUserRedactOthers: context.viewState.canCurrentUserRedactOthers,
canCurrentUserPin: context.viewState.canCurrentUserPin,
pinnedEvents: context.viewState.pinnedEventsState.pinnedEvents.set,
pinnedEventIDs: context.viewState.pinnedEventsState.pinnedEventIDs.set,
isDM: context.viewState.isEncryptedOneToOneRoom,
isViewSourceEnabled: context.viewState.isViewSourceEnabled)
TimelineItemMacContextMenu(item: timelineItem, actionProvider: provider) { action in
Expand Down
2 changes: 1 addition & 1 deletion ElementX/Sources/Services/Room/RoomProxy.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ class RoomProxy: RoomProxyProtocol {
}
}

var pinnedEvents: [String] {
var pinnedEventIDs: [String] {
get async {
await (try? room.roomInfo().pinnedEventIds) ?? []

Check failure on line 91 in ElementX/Sources/Services/Room/RoomProxy.swift

View workflow job for this annotation

GitHub Actions / Tests (Enterprise) 

value of type 'RoomInfo' has no member 'pinnedEventIds'
}
Expand Down
2 changes: 1 addition & 1 deletion ElementX/Sources/Services/Room/RoomProxyProtocol.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ protocol RoomProxyProtocol {
var isSpace: Bool { get }
var isEncrypted: Bool { get }
var isFavourite: Bool { get async }
var pinnedEvents: [String] { get async }
var pinnedEventIDs: [String] { get async }
var membership: Membership { get }
var hasOngoingCall: Bool { get }
var canonicalAlias: String? { get }
Expand Down

0 comments on commit dc40f0d

Please sign in to comment.