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

Only use device type name as fallback for session display name #6820

Merged
merged 9 commits into from
Oct 7, 2022
5 changes: 5 additions & 0 deletions Riot/Assets/en.lproj/Vector.strings
Original file line number Diff line number Diff line change
Expand Up @@ -2441,6 +2441,11 @@ To enable access, tap Settings> Location and select Always";
"device_name_mobile" = "%@ Mobile";
"device_name_unknown" = "Unknown client";

"device_type_name_desktop" = "Desktop";
"device_type_name_web" = "Web";
"device_type_name_mobile" = "Mobile";
"device_type_name_unknown" = "Unknown";

"user_session_details_title" = "Session details";
"user_session_details_session_section_header" = "Session";
"user_session_details_application_section_header" = "Application";
Expand Down
5 changes: 5 additions & 0 deletions Riot/Categories/String.swift
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,11 @@ extension String {
func vc_reversed() -> String {
return String(self.reversed())
}

/// Returns nil if the string is empty or the string itself otherwise
func vc_nilIfEmpty() -> String? {
isEmpty ? nil : self
}
}

extension Optional where Wrapped == String {
Expand Down
16 changes: 16 additions & 0 deletions Riot/Generated/Strings.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1575,6 +1575,22 @@ public class VectorL10n: NSObject {
public static func deviceNameWeb(_ p1: String) -> String {
return VectorL10n.tr("Vector", "device_name_web", p1)
}
/// Desktop
public static var deviceTypeNameDesktop: String {
return VectorL10n.tr("Vector", "device_type_name_desktop")
}
/// Mobile
public static var deviceTypeNameMobile: String {
return VectorL10n.tr("Vector", "device_type_name_mobile")
}
/// Unknown
public static var deviceTypeNameUnknown: String {
return VectorL10n.tr("Vector", "device_type_name_unknown")
}
/// Web
public static var deviceTypeNameWeb: String {
return VectorL10n.tr("Vector", "device_type_name_web")
}
/// The other party cancelled the verification.
public static var deviceVerificationCancelled: String {
return VectorL10n.tr("Vector", "device_verification_cancelled")
Expand Down
10 changes: 4 additions & 6 deletions RiotSwiftUI/Modules/UserSessions/Common/DeviceType.swift
Original file line number Diff line number Diff line change
Expand Up @@ -38,17 +38,15 @@ enum DeviceType {
}

var name: String {
let appName = AppInfo.current.displayName
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pixlwave sorry just pinging to inform you that I've decided to include this in the current PR as it's related. We shouldn't use the current app name here because the device type name is used for all sessions, so there's no guarantee the others are Element, too.


switch self {
case .desktop:
return VectorL10n.deviceNameDesktop(appName)
return VectorL10n.deviceTypeNameDesktop
case .web:
return VectorL10n.deviceNameWeb(appName)
return VectorL10n.deviceTypeNameWeb
case .mobile:
return VectorL10n.deviceNameMobile(appName)
return VectorL10n.deviceTypeNameMobile
case .unknown:
return VectorL10n.deviceNameUnknown
return VectorL10n.deviceTypeNameUnknown
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
//
// Copyright 2022 New Vector Ltd
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
//

import XCTest

@testable import RiotSwiftUI

class UserSessionNameFormatterTests: XCTestCase {
func testSessionDisplayNameTrumpsDeviceTypeName() {
XCTAssertEqual("Johnny's iPhone", UserSessionNameFormatter.sessionName(deviceType: .mobile, sessionDisplayName: "Johnny's iPhone"))
}

func testEmptySessionDisplayNameFallsBackToDeviceTypeName() {
XCTAssertEqual(DeviceType.mobile.name, UserSessionNameFormatter.sessionName(deviceType: .mobile, sessionDisplayName: ""))
}

func testNilSessionDisplayNameFallsBackToDeviceTypeName() {
XCTAssertEqual(DeviceType.mobile.name, UserSessionNameFormatter.sessionName(deviceType: .mobile, sessionDisplayName: nil))
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,6 @@ import Foundation
enum UserSessionNameFormatter {
/// Session name with client name and session display name
static func sessionName(deviceType: DeviceType, sessionDisplayName: String?) -> String {
let sessionName: String

let clientName = deviceType.name

if let sessionDisplayName = sessionDisplayName {
sessionName = VectorL10n.userSessionName(clientName, sessionDisplayName)
} else {
sessionName = clientName
}

return sessionName
sessionDisplayName?.vc_nilIfEmpty() ?? deviceType.name
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class UserOtherSessionsUITests: MockScreenTestCase {
func test_whenOtherSessionsWithInactiveSessionFilterPresented_correctItemsDisplayed() {
app.goToScreenWithIdentifier(MockUserOtherSessionsScreenState.inactiveSessions.title)

XCTAssertTrue(app.buttons["RiotSwiftUI Mobile: iOS, Inactive for 90+ days"].exists)
XCTAssertTrue(app.buttons["iOS, Inactive for 90+ days"].exists)
}

func test_whenOtherSessionsWithUnverifiedSessionFilterPresented_correctHeaderDisplayed() {
Expand All @@ -41,6 +41,6 @@ class UserOtherSessionsUITests: MockScreenTestCase {
func test_whenOtherSessionsWithUnverifiedSessionFilterPresented_correctItemsDisplayed() {
app.goToScreenWithIdentifier(MockUserOtherSessionsScreenState.unverifiedSessions.title)

XCTAssertTrue(app.buttons["RiotSwiftUI Mobile: iOS, Unverified · Your current session"].exists)
XCTAssertTrue(app.buttons["iOS, Unverified · Your current session"].exists)
}
}
6 changes: 6 additions & 0 deletions RiotTests/String+Element.swift
Original file line number Diff line number Diff line change
Expand Up @@ -45,4 +45,10 @@ class String_Element: XCTestCase {
let string3 = "ab"
XCTAssertEqual(string3.vc_reversed(), "ba")
}

func testNilIfEmpty() {
XCTAssertNil("".vc_nilIfEmpty())
XCTAssertNotNil(" ".vc_nilIfEmpty())
XCTAssertNotNil("Johnny was here".vc_nilIfEmpty())
}
}
1 change: 1 addition & 0 deletions changelog.d/6820.change
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Only use device type name as fallback for session display name