Skip to content

PM-10084: Move save button into navigation bar #775

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

Merged
merged 2 commits into from
Jul 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
17 changes: 4 additions & 13 deletions BitwardenShared/UI/Auth/Landing/SelfHosted/SelfHostedView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ struct SelfHostedView: View {
VStack(spacing: 16) {
selfHostedEnvironment
customEnvironment
saveButton
}
.textFieldConfiguration(.url)
.navigationBar(title: Localizations.settings, titleDisplayMode: .inline)
Expand All @@ -25,6 +24,10 @@ struct SelfHostedView: View {
cancelToolbarItem {
store.send(.dismiss)
}

saveToolbarItem {
await store.perform(.saveEnvironment)
}
}
}

Expand Down Expand Up @@ -77,18 +80,6 @@ struct SelfHostedView: View {
.padding(.top, 8)
}

/// The save button.
private var saveButton: some View {
AsyncButton {
await store.perform(.saveEnvironment)
} label: {
Text(Localizations.save)
}
.accessibilityIdentifier("SaveButton")
.buttonStyle(.primary())
.padding(.top, 8)
}

/// The self-hosted environment section.
private var selfHostedEnvironment: some View {
section(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,47 @@ import XCTest
@testable import BitwardenShared

class SelfHostedViewTests: BitwardenTestCase {
let subject = SelfHostedView(store: Store(processor: StateProcessor(state: SelfHostedState())))
// MARK: Properties

var processor: MockProcessor<SelfHostedState, SelfHostedAction, SelfHostedEffect>!
var subject: SelfHostedView!

// MARK: Setup & Teardown

override func setUp() {
super.setUp()

processor = MockProcessor(state: SelfHostedState())

subject = SelfHostedView(store: Store(processor: processor))
}

override func tearDown() {
super.tearDown()

subject = nil
}

// MARK: Tests

/// Tapping the cancel button dispatches the `.dismiss` action.
func test_cancelButton_tap() throws {
let button = try subject.inspect().find(button: Localizations.cancel)
try button.tap()
XCTAssertEqual(processor.dispatchedActions.last, .dismiss)
}

/// Tapping the save button dispatches the `.saveEnvironment` action.
func test_saveButton_tap() async throws {
let button = try subject.inspect().find(asyncButton: Localizations.save)
try await button.tap()
XCTAssertEqual(processor.effects.last, .saveEnvironment)
}

// MARK: Snapshots

/// Tests that the view renders correctly.
func test_viewRender() {
assertSnapshot(of: subject, as: .defaultPortrait)
assertSnapshot(of: subject.navStackWrapped, as: .defaultPortrait)
}
}
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,16 @@ extension View {
.accessibilityIdentifier("EditItemButton")
}

/// Returns a toolbar button configured for saving an item.
///
/// - Parameter action: The action to perform when the save button is tapped.
/// - Returns: A `Button` configured for saving an item.
///
func saveToolbarButton(action: @escaping () async -> Void) -> some View {
toolbarButton(Localizations.save, action: action)
.accessibilityIdentifier("SaveButton")
}

/// Returns a `Button` that displays an image for use in a toolbar.
///
/// - Parameters:
Expand Down Expand Up @@ -140,4 +150,15 @@ extension View {
optionsToolbarMenu(content: content)
}
}

/// A `ToolbarItem` for views with a save button.
///
/// - Parameter action: The action to perform when the save button is tapped.
/// - Returns: A `ToolbarItem` with a save button.
///
func saveToolbarItem(_ action: @escaping () async -> Void) -> some ToolbarContent {
ToolbarItem(placement: .topBarTrailing) {
saveToolbarButton(action: action)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ struct AddEditFolderView: View {
cancelToolbarItem {
store.send(.dismiss)
}

saveToolbarItem {
await store.perform(.saveTapped)
}
}
}

Expand All @@ -39,24 +43,28 @@ struct AddEditFolderView: View {
content
.navigationBar(title: Localizations.editFolder, titleDisplayMode: .inline)
.toolbar {
optionsToolbarItem {
AsyncButton(Localizations.delete, role: .destructive) {
await store.perform(.deleteTapped)
}
}

cancelToolbarItem {
store.send(.dismiss)
}

ToolbarItemGroup(placement: .topBarTrailing) {
saveToolbarButton {
await store.perform(.saveTapped)
}

optionsToolbarMenu {
AsyncButton(Localizations.delete, role: .destructive) {
await store.perform(.deleteTapped)
}
}
}
}
}

/// The content of the view in either mode.
private var content: some View {
VStack(alignment: .leading, spacing: 20) {
nameEntryTextField

saveButton
}
.scrollView()
}
Expand All @@ -71,13 +79,4 @@ struct AddEditFolderView: View {
)
)
}

/// The save button.
private var saveButton: some View {
AsyncButton(Localizations.save) {
await store.perform(.saveTapped)
}
.accessibilityIdentifier("SaveButton")
.buttonStyle(.primary())
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -61,31 +61,48 @@ class AddEditFolderViewTests: BitwardenTestCase {
XCTAssertEqual(processor.dispatchedActions.last, .folderNameTextChanged("text"))
}

/// Tapping the save button performs the `.saveTapped` effect.
func test_saveButton_tap() async throws {
/// Tapping the save button in add mode performs the `.saveTapped` effect.
func test_saveButton_tapAdd() async throws {
let button = try subject.inspect().find(asyncButton: Localizations.save)
try await button.tap()

XCTAssertEqual(processor.effects.last, .saveTapped)
}

/// Tapping the save button in edit mode performs the `.saveTapped` effect.
func test_saveButton_tapEdit() async throws {
processor.state.mode = .edit(.fixture())
let button = try subject.inspect().find(asyncButton: Localizations.save)
try await button.tap()
XCTAssertEqual(processor.effects.last, .saveTapped)
}

// MARK: Snapshots

/// Tests the view renders correctly when the text field is empty.
func test_snapshot_add_empty() {
assertSnapshots(matching: subject, as: [.defaultPortrait, .defaultPortraitDark, .defaultPortraitAX5])
assertSnapshots(
matching: subject.navStackWrapped,
as: [.defaultPortrait, .defaultPortraitDark, .defaultPortraitAX5]
)
}

/// Tests the view renders correctly when the text field is populated.
func test_snapshot_add_populated() {
processor.state.folderName = "Super cool folder name"
assertSnapshots(matching: subject, as: [.defaultPortrait, .defaultPortraitDark, .defaultPortraitAX5])
assertSnapshots(
matching: subject.navStackWrapped,
as: [.defaultPortrait, .defaultPortraitDark, .defaultPortraitAX5]
)
}

/// Tests the view renders correctly when the text field is populated.
func test_snapshot_edit_populated() {
processor.state.mode = .edit(.fixture())
processor.state.folderName = "Super cool folder name"
assertSnapshots(matching: subject, as: [.defaultPortrait, .defaultPortraitDark, .defaultPortraitAX5])
assertSnapshots(
matching: subject.navStackWrapped,
as: [.defaultPortrait, .defaultPortraitDark, .defaultPortraitAX5]
)
}
}
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,14 @@ struct GeneratorState: Equatable {
}
}

/// A flag indicating if the options toolbar button is visible.
var isOptionsButtonVisible: Bool {
switch self {
case .tab: true
case .inPlace: false
}
}

/// A flag indicating if the select button is visible.
var isSelectButtonVisible: Bool {
switch self {
Expand Down
27 changes: 14 additions & 13 deletions BitwardenShared/UI/Tools/Generator/Generator/GeneratorView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -25,19 +25,11 @@ struct GeneratorView: View {
ForEach(store.state.formSections) { section in
sectionView(section)
}

if store.state.presentationMode.isSelectButtonVisible {
Button(Localizations.select) {
store.send(.selectButtonPressed)
}
.buttonStyle(.primary())
.accessibilityIdentifier("SelectButton")
}
}
.padding(16)
}
.background(Asset.Colors.backgroundSecondary.swiftUIColor)
.navigationBarTitleDisplayMode(.large)
.navigationBarTitleDisplayMode(store.state.presentationMode == .inPlace ? .inline : .large)
.navigationTitle(Localizations.generator)
.task { await store.perform(.appeared) }
.onChange(of: focusedFieldKeyPath) { newValue in
Expand All @@ -56,10 +48,19 @@ struct GeneratorView: View {
}
}

ToolbarItem(placement: .topBarTrailing) {
optionsToolbarMenu {
Button(Localizations.passwordHistory) {
store.send(.showPasswordHistory)
ToolbarItemGroup(placement: .topBarTrailing) {
if store.state.presentationMode.isSelectButtonVisible {
toolbarButton(Localizations.select) {
store.send(.selectButtonPressed)
}
.accessibilityIdentifier("SelectButton")
}

if store.state.presentationMode.isOptionsButtonVisible {
optionsToolbarMenu {
Button(Localizations.passwordHistory) {
store.send(.showPasswordHistory)
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,10 +96,10 @@ class GeneratorViewTests: BitwardenTestCase {
}

/// Tapping the select button dispatches the `.selectButtonPressed` action.
func test_selectButton_tap() throws {
func test_selectButton_tap() async throws {
processor.state.presentationMode = .inPlace
let button = try subject.inspect().find(button: Localizations.select)
try button.tap()
let button = try subject.inspect().find(asyncButton: Localizations.select)
try await button.tap()
XCTAssertEqual(processor.dispatchedActions.last, .selectButtonPressed)
}

Expand Down Expand Up @@ -180,7 +180,7 @@ class GeneratorViewTests: BitwardenTestCase {
processor.state.generatedValue = "pa11w0rd"
processor.state.showCopiedValueToast()
assertSnapshot(
matching: subject,
matching: subject.navStackWrapped,
as: .defaultPortrait
)
}
Expand All @@ -189,7 +189,7 @@ class GeneratorViewTests: BitwardenTestCase {
func test_snapshot_generatorViewPassphrase() {
processor.state.passwordState.passwordGeneratorType = .passphrase
assertSnapshot(
matching: subject,
matching: subject.navStackWrapped,
as: .defaultPortrait
)
}
Expand All @@ -198,7 +198,7 @@ class GeneratorViewTests: BitwardenTestCase {
func test_snapshot_generatorViewPassword() {
processor.state.passwordState.passwordGeneratorType = .password
assertSnapshot(
matching: subject,
matching: subject.navStackWrapped,
as: .defaultPortrait
)
}
Expand All @@ -207,14 +207,14 @@ class GeneratorViewTests: BitwardenTestCase {
func test_snapshot_generatorViewPassword_inPlace() {
processor.state.passwordState.passwordGeneratorType = .password
processor.state.presentationMode = .inPlace
assertSnapshot(of: subject, as: .tallPortrait)
assertSnapshot(of: subject.navStackWrapped, as: .tallPortrait)
}

/// Test a snapshot of the password generation view with a policy in effect.
func test_snapshot_generatorViewPassword_policyInEffect() {
processor.state.isPolicyInEffect = true
assertSnapshot(
matching: subject,
matching: subject.navStackWrapped,
as: .defaultPortrait
)
}
Expand All @@ -224,7 +224,7 @@ class GeneratorViewTests: BitwardenTestCase {
processor.state.generatorType = .username
processor.state.usernameState.usernameGeneratorType = .catchAllEmail
assertSnapshot(
matching: subject,
matching: subject.navStackWrapped,
as: .defaultPortrait
)
}
Expand All @@ -234,7 +234,7 @@ class GeneratorViewTests: BitwardenTestCase {
processor.state.generatorType = .username
processor.state.usernameState.usernameGeneratorType = .forwardedEmail
assertSnapshot(
matching: subject,
matching: subject.navStackWrapped,
as: .defaultPortrait
)
}
Expand All @@ -244,7 +244,7 @@ class GeneratorViewTests: BitwardenTestCase {
processor.state.generatorType = .username
processor.state.usernameState.usernameGeneratorType = .plusAddressedEmail
assertSnapshot(
matching: subject,
matching: subject.navStackWrapped,
as: .defaultPortrait
)
}
Expand All @@ -254,15 +254,15 @@ class GeneratorViewTests: BitwardenTestCase {
processor.state.generatorType = .username
processor.state.usernameState.usernameGeneratorType = .plusAddressedEmail
processor.state.presentationMode = .inPlace
assertSnapshot(matching: subject, as: .defaultPortrait)
assertSnapshot(matching: subject.navStackWrapped, as: .defaultPortrait)
}

/// Test a snapshot of the random word username generation view.
func test_snapshot_generatorViewUsernameRandomWord() {
processor.state.generatorType = .username
processor.state.usernameState.usernameGeneratorType = .randomWord
assertSnapshot(
matching: subject,
matching: subject.navStackWrapped,
as: .defaultPortrait
)
}
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Loading