Skip to content

Commit fc4e4a8

Browse files
LRNcardozoWDFLRNcardozoWDF
committed
pm-19305 Fix pr comments
1 parent 1077368 commit fc4e4a8

File tree

5 files changed

+88
-100
lines changed

5 files changed

+88
-100
lines changed

‎BitwardenKit/UI/Platform/Application/Views/SettingsPickerField.swift‎

Lines changed: 8 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,6 @@ public struct SettingsPickerField: View {
1717
/// The footer text displayed below the toggle.
1818
let footer: String?
1919

20-
/// Whether the menu field should have a bottom divider.
21-
let hasDivider: Bool
22-
2320
/// The date picker value.
2421
@Binding var pickerValue: Int
2522

@@ -57,7 +54,7 @@ public struct SettingsPickerField: View {
5754
.padding(.horizontal, 16)
5855
}
5956

60-
if hasDivider {
57+
if footer != nil {
6158
Divider()
6259
.padding(.leading, 16)
6360
}
@@ -66,21 +63,17 @@ public struct SettingsPickerField: View {
6663
CountdownDatePicker(duration: $pickerValue)
6764
.frame(maxWidth: .infinity)
6865

69-
if hasDivider {
66+
if footer != nil {
7067
Divider()
7168
.padding(.leading, 16)
7269
}
7370
}
7471

75-
if footer != nil {
76-
Group {
77-
if let footer {
78-
Text(footer)
79-
.styleGuide(.subheadline)
80-
.foregroundColor(Color(asset: SharedAsset.Colors.textSecondary))
81-
}
82-
}
83-
.padding(EdgeInsets(top: 12, leading: 16, bottom: 12, trailing: 16))
72+
if let footer {
73+
Text(footer)
74+
.styleGuide(.subheadline)
75+
.foregroundColor(Color(asset: SharedAsset.Colors.textSecondary))
76+
.padding(EdgeInsets(top: 12, leading: 16, bottom: 12, trailing: 16))
8477
}
8578
}
8679
.background(SharedAsset.Colors.backgroundSecondary.swiftUIColor)
@@ -95,21 +88,18 @@ public struct SettingsPickerField: View {
9588
/// - footer: The footer text displayed below the menu field.
9689
/// - customTimeoutValue: The custom session timeout value.
9790
/// - pickerValue: The date picker value.
98-
/// - hasDivider: Whether or not the field has a bottom edge divider.
9991
/// - customTimeoutAccessibilityLabel: The accessibility label used for the custom timeout value.
10092
///
10193
public init(
10294
title: String,
10395
footer: String? = nil,
10496
customTimeoutValue: String,
10597
pickerValue: Binding<Int>,
106-
hasDivider: Bool = true,
107-
customTimeoutAccessibilityLabel: String,
98+
customTimeoutAccessibilityLabel: String
10899
) {
109100
self.customTimeoutAccessibilityLabel = customTimeoutAccessibilityLabel
110101
self.customTimeoutValue = customTimeoutValue
111102
self.footer = footer
112-
self.hasDivider = hasDivider
113103
_pickerValue = pickerValue
114104
self.title = title
115105
}

‎BitwardenShared/Core/Vault/Services/PolicyService.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ protocol PolicyService: AnyObject {
1616
/// If the policy for a maximum vault timeout value is enabled,
1717
/// return the value and action to take upon timeout.
1818
///
19-
/// - Returns: The timeout value in minutes, and the action to take upon timeout.
19+
/// - Returns: The session timeout policy.
2020
///
2121
func fetchTimeoutPolicyValues() async throws -> SessionTimeoutPolicy?
2222

‎BitwardenShared/Core/Vault/Services/SyncServiceTests.swift‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -177,8 +177,7 @@ class SyncServiceTests: BitwardenTestCase {
177177
XCTAssertEqual(stateService.vaultTimeout["1"], SessionTimeoutValue(rawValue: 60))
178178
}
179179

180-
/// `fetchSync()` updates the user's timeout action and value
181-
/// if the user's timeout value is greater than the policy's.
180+
/// `fetchSync()` updates the user's timeout action and ignores the time value.
182181
func test_checkVaultTimeoutPolicy_setActionForOnAppRestartType() async throws {
183182
client.result = .httpSuccess(testData: .syncWithCiphers)
184183
stateService.activeAccount = .fixture()
@@ -195,6 +194,7 @@ class SyncServiceTests: BitwardenTestCase {
195194
try await subject.fetchSync(forceSync: false)
196195

197196
XCTAssertEqual(stateService.timeoutAction["1"], .logout)
197+
XCTAssertEqual(stateService.vaultTimeout["1"], SessionTimeoutValue(rawValue: 120))
198198
}
199199

200200
/// `fetchSync()` updates the user's timeout action and value - if the timeout value is set to

‎BitwardenShared/UI/Platform/Settings/Settings/AccountSecurity/AccountSecurityState.swift‎

Lines changed: 43 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -134,19 +134,46 @@ struct AccountSecurityState: Equatable {
134134
customTimeoutValueSeconds.timeInHoursMinutes(shouldSpellOut: true)
135135
}
136136

137-
/// The custom session timeout value, in seconds, initially set to 60 seconds.
138-
var customTimeoutValueSeconds: Int {
139-
guard case let .custom(customValueInMinutes) = sessionTimeoutValue, customValueInMinutes > 0 else {
140-
return 60
137+
/// The message informing the user of their organization's policy-set session timeout duration.
138+
var customTimeoutMessage: String {
139+
switch (policyTimeoutHours, policyTimeoutMinutes) {
140+
case let (hours, minutes) where hours > 0 && minutes > 0:
141+
Localizations.yourOrganizationHasSetTheDefaultSessionTimeoutToXAndY(
142+
Localizations.xHours(
143+
policyTimeoutHours,
144+
),
145+
Localizations.xMinutes(
146+
policyTimeoutMinutes,
147+
),
148+
)
149+
case let (hours, _) where hours > 0:
150+
Localizations.yourOrganizationHasSetTheDefaultSessionTimeoutToX(
151+
Localizations.xHours(
152+
policyTimeoutHours,
153+
),
154+
)
155+
default:
156+
Localizations.yourOrganizationHasSetTheDefaultSessionTimeoutToX(
157+
Localizations.xMinutes(
158+
policyTimeoutMinutes,
159+
),
160+
)
141161
}
142-
return customValueInMinutes * 60
143162
}
144163

145164
/// The string representation of the custom session timeout value.
146165
var customTimeoutString: String {
147166
customTimeoutValueSeconds.timeInHoursMinutes()
148167
}
149168

169+
/// The custom session timeout value, in seconds, initially set to 60 seconds.
170+
var customTimeoutValueSeconds: Int {
171+
guard case let .custom(customValueInMinutes) = sessionTimeoutValue, customValueInMinutes > 0 else {
172+
return 60
173+
}
174+
return customValueInMinutes * 60
175+
}
176+
150177
/// Whether the user has a method to unlock the vault (master password, pin set, or biometrics
151178
/// enabled).
152179
var hasUnlockMethod: Bool {
@@ -185,17 +212,6 @@ struct AccountSecurityState: Equatable {
185212
return Localizations.thisSettingIsManagedByYourOrganization
186213
}
187214

188-
/// The policy's timeout value in hours.
189-
var policyTimeoutHours: Int {
190-
policyTimeoutValue / 60
191-
}
192-
193-
/// The message to display if a timeout policy is in effect for the user.
194-
var policyTimeoutMessage: String? {
195-
guard !isShowingCustomTimeout else { return nil }
196-
return policyTimeoutCustomMessage
197-
}
198-
199215
/// The message to display if a timeout policy is in effect for the user.
200216
var policyTimeoutCustomMessage: String? {
201217
guard isPolicyTimeoutEnabled, let policy = policyTimeoutType else { return nil }
@@ -213,6 +229,17 @@ struct AccountSecurityState: Equatable {
213229
}
214230
}
215231

232+
/// The policy's timeout value in hours.
233+
var policyTimeoutHours: Int {
234+
policyTimeoutValue / 60
235+
}
236+
237+
/// The message to display if a timeout policy is in effect for the user.
238+
var policyTimeoutMessage: String? {
239+
guard !isShowingCustomTimeout else { return nil }
240+
return policyTimeoutCustomMessage
241+
}
242+
216243
/// The policy's timeout value in minutes.
217244
var policyTimeoutMinutes: Int {
218245
policyTimeoutValue % 60
@@ -231,32 +258,6 @@ struct AccountSecurityState: Equatable {
231258
!removeUnlockWithPinPolicyEnabled || isUnlockWithPINCodeOn
232259
}
233260

234-
var customTimeoutMessage: String {
235-
switch (policyTimeoutHours, policyTimeoutMinutes) {
236-
case let (hours, minutes) where hours > 0 && minutes > 0:
237-
Localizations.yourOrganizationHasSetTheDefaultSessionTimeoutToXAndY(
238-
Localizations.xHours(
239-
policyTimeoutHours,
240-
),
241-
Localizations.xMinutes(
242-
policyTimeoutMinutes,
243-
),
244-
)
245-
case let (hours, _) where hours > 0:
246-
Localizations.yourOrganizationHasSetTheDefaultSessionTimeoutToX(
247-
Localizations.xHours(
248-
policyTimeoutHours,
249-
),
250-
)
251-
default:
252-
Localizations.yourOrganizationHasSetTheDefaultSessionTimeoutToX(
253-
Localizations.xMinutes(
254-
policyTimeoutMinutes,
255-
),
256-
)
257-
}
258-
}
259-
260261
/// Returns the available timeout options based on policy type and value.
261262
///
262263
/// - Parameters:

‎BitwardenShared/UI/Platform/Settings/Settings/AccountSecurity/AccountSecurityView.swift‎

Lines changed: 34 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -146,48 +146,45 @@ struct AccountSecurityView: View {
146146
/// The session timeout section.
147147
private var sessionTimeoutSection: some View {
148148
SectionView(Localizations.sessionTimeout, contentSpacing: 8) {
149-
VStack(spacing: 16) {
150-
ContentBlock(dividerLeadingPadding: 16) {
151-
BitwardenMenuField(
152-
title: Localizations.sessionTimeout,
153-
footer: store.state.policyTimeoutMessage,
154-
accessibilityIdentifier: "VaultTimeoutChooser",
155-
options: store.state.availableTimeoutOptions,
156-
selection: store.binding(
157-
get: \.sessionTimeoutValue,
158-
send: AccountSecurityAction.sessionTimeoutValueChanged,
159-
),
160-
)
161-
.disabled(store.state.isSessionTimeoutPickerDisabled)
149+
ContentBlock(dividerLeadingPadding: 16) {
150+
BitwardenMenuField(
151+
title: Localizations.sessionTimeout,
152+
footer: store.state.policyTimeoutMessage,
153+
accessibilityIdentifier: "VaultTimeoutChooser",
154+
options: store.state.availableTimeoutOptions,
155+
selection: store.binding(
156+
get: \.sessionTimeoutValue,
157+
send: AccountSecurityAction.sessionTimeoutValueChanged,
158+
),
159+
)
160+
.disabled(store.state.isSessionTimeoutPickerDisabled)
162161

163-
if store.state.isShowingCustomTimeout {
164-
SettingsPickerField(
165-
title: "",
166-
footer: store.state.policyTimeoutCustomMessage,
167-
customTimeoutValue: store.state.customTimeoutString,
168-
pickerValue: store.binding(
169-
get: \.customTimeoutValueSeconds,
170-
send: AccountSecurityAction.customTimeoutValueSecondsChanged,
171-
),
172-
hasDivider: true,
173-
customTimeoutAccessibilityLabel: store.state.customTimeoutAccessibilityLabel,
174-
)
175-
}
176-
}
177-
ContentBlock(dividerLeadingPadding: 16) {
178-
BitwardenMenuField(
179-
title: Localizations.sessionTimeoutAction,
180-
footer: store.state.policyTimeoutActionMessage,
181-
accessibilityIdentifier: "VaultTimeoutActionChooser",
182-
options: store.state.availableTimeoutActions,
183-
selection: store.binding(
184-
get: \.sessionTimeoutAction,
185-
send: AccountSecurityAction.sessionTimeoutActionChanged,
162+
if store.state.isShowingCustomTimeout {
163+
SettingsPickerField(
164+
title: "",
165+
footer: store.state.policyTimeoutCustomMessage,
166+
customTimeoutValue: store.state.customTimeoutString,
167+
pickerValue: store.binding(
168+
get: \.customTimeoutValueSeconds,
169+
send: AccountSecurityAction.customTimeoutValueSecondsChanged,
186170
),
171+
customTimeoutAccessibilityLabel: store.state.customTimeoutAccessibilityLabel,
187172
)
188-
.disabled(store.state.isSessionTimeoutActionDisabled)
189173
}
190174
}
175+
ContentBlock(dividerLeadingPadding: 16) {
176+
BitwardenMenuField(
177+
title: Localizations.sessionTimeoutAction,
178+
footer: store.state.policyTimeoutActionMessage,
179+
accessibilityIdentifier: "VaultTimeoutActionChooser",
180+
options: store.state.availableTimeoutActions,
181+
selection: store.binding(
182+
get: \.sessionTimeoutAction,
183+
send: AccountSecurityAction.sessionTimeoutActionChanged,
184+
),
185+
)
186+
.disabled(store.state.isSessionTimeoutActionDisabled)
187+
}
191188
}
192189
}
193190

0 commit comments

Comments
 (0)