From 9ca60bc70bde1277e7719967c19753c167baabbf Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sat, 30 May 2026 15:50:59 +0000 Subject: [PATCH 1/2] POI: hide Attributes for local objects; All Tags header tap targets - Treat pending geometry (ident < 0) like new nodes when restoring poiTabIndex: strip Attributes tab and clamp saved index 2 to Common Tags. - All Tags preset header: title switches to Common Tags; chevron opens picker. Co-authored-by: Tobias --- src/iOS/POI/POIAllTagsViewController.swift | 83 ++++++++++++++++------ src/iOS/POI/POITabBarController.swift | 47 ++++-------- 2 files changed, 78 insertions(+), 52 deletions(-) diff --git a/src/iOS/POI/POIAllTagsViewController.swift b/src/iOS/POI/POIAllTagsViewController.swift index 5066327d7..1140ab84a 100644 --- a/src/iOS/POI/POIAllTagsViewController.swift +++ b/src/iOS/POI/POIAllTagsViewController.swift @@ -13,8 +13,8 @@ private let EDIT_RELATIONS = false private class SectionHeaderCell: UITableViewHeaderFooterView { static let reuseIdentifier = "SectionHeaderCell" - let label = UILabel() - let button = UIButton() + let titleButton = UIButton(type: .system) + let changeFeatureButton = UIButton(type: .system) override init(reuseIdentifier: String?) { super.init(reuseIdentifier: reuseIdentifier) @@ -27,32 +27,75 @@ private class SectionHeaderCell: UITableViewHeaderFooterView { } func configureContents() { - label.translatesAutoresizingMaskIntoConstraints = false - button.translatesAutoresizingMaskIntoConstraints = false + titleButton.translatesAutoresizingMaskIntoConstraints = false + changeFeatureButton.translatesAutoresizingMaskIntoConstraints = false - if #available(iOS 13.0, *) { - label.textColor = UIColor.secondaryLabel + titleButton.contentHorizontalAlignment = .leading + titleButton.addTarget(self, action: #selector(showCommonTagsTab(_:)), for: .touchUpInside) + titleButton.accessibilityHint = NSLocalizedString("Shows Common Tags", comment: "") + + changeFeatureButton.accessibilityLabel = NSLocalizedString("Change preset", comment: "") + changeFeatureButton.accessibilityHint = NSLocalizedString("Opens the preset picker", comment: "") + changeFeatureButton.addTarget(self, action: #selector(pickFeature(_:)), for: .touchUpInside) + + if #available(iOS 15.0, *) { + var titleConfig = UIButton.Configuration.plain() + if #available(iOS 13.0, *) { + titleConfig.baseForegroundColor = UIColor.secondaryLabel + } else { + titleConfig.baseForegroundColor = UIColor.darkGray + } + titleConfig.contentInsets = .zero + titleButton.configuration = titleConfig + + var featureConfig = UIButton.Configuration.plain() + featureConfig.image = UIImage(systemName: "chevron.right") + featureConfig.baseForegroundColor = UIColor.systemBlue + changeFeatureButton.configuration = featureConfig } else { - label.textColor = UIColor.darkGray + if #available(iOS 13.0, *) { + titleButton.setTitleColor(UIColor.secondaryLabel, for: .normal) + changeFeatureButton.setImage(UIImage(systemName: "chevron.right"), for: .normal) + } else { + titleButton.setTitleColor(UIColor.darkGray, for: .normal) + changeFeatureButton.setTitle(">", for: .normal) + } + changeFeatureButton.tintColor = UIColor.systemBlue } - button.setTitle(">", for: .normal) - button.setTitleColor(UIColor.systemBlue, for: .normal) - button.addTarget(self, action: #selector(pickFeature(_:)), for: .touchUpInside) - contentView.addSubview(label) - contentView.addSubview(button) + contentView.addSubview(titleButton) + contentView.addSubview(changeFeatureButton) NSLayoutConstraint.activate([ - label.centerYAnchor.constraint(equalTo: contentView.centerYAnchor), - label.leadingAnchor.constraint(equalToSystemSpacingAfter: contentView.leadingAnchor, multiplier: 1.0), - label.trailingAnchor.constraint(greaterThanOrEqualTo: button.leadingAnchor, constant: 10.0), - - button.centerYAnchor.constraint(equalTo: contentView.centerYAnchor), - button.trailingAnchor.constraint(equalTo: contentView.layoutMarginsGuide.trailingAnchor, constant: 10.0), - button.widthAnchor.constraint(equalToConstant: 44.0) + titleButton.centerYAnchor.constraint(equalTo: contentView.centerYAnchor), + titleButton.leadingAnchor.constraint(equalToSystemSpacingAfter: contentView.leadingAnchor, multiplier: 1.0), + titleButton.trailingAnchor.constraint(greaterThanOrEqualTo: changeFeatureButton.leadingAnchor, constant: 10.0), + + changeFeatureButton.centerYAnchor.constraint(equalTo: contentView.centerYAnchor), + changeFeatureButton.trailingAnchor.constraint(equalTo: contentView.layoutMarginsGuide.trailingAnchor, constant: 10.0), + changeFeatureButton.widthAnchor.constraint(equalToConstant: 44.0), + changeFeatureButton.heightAnchor.constraint(equalToConstant: 44.0) ]) } + func setPresetTitle(_ text: String) { + if #available(iOS 15.0, *) { + var config = titleButton.configuration ?? .plain() + config.title = text + titleButton.configuration = config + } else { + titleButton.setTitle(text, for: .normal) + } + titleButton.accessibilityLabel = text + } + + @objc func showCommonTagsTab(_ sender: Any?) { + guard let tabBar = tabBarController as? POITabBarController else { return } + UserPrefs.shared.poiTabIndex.value = 0 + guard tabBar.selectedIndex != 0 else { return } + tabBar.slideTabTo(tabIndex: 0) + } + @objc func pickFeature(_ sender: Any?) { var r: UIResponder = self while true { @@ -467,7 +510,7 @@ class POIAllTagsViewController: UITableViewController, POIFeaturePickerDelegate, let cell: SectionHeaderCell = tableView .dequeueReusableHeaderFooterView(withIdentifier: SectionHeaderCell .reuseIdentifier) as! SectionHeaderCell - cell.label.text = currentFeature?.localizedName.uppercased() ?? "TAGS" + cell.setPresetTitle(currentFeature?.localizedName.uppercased() ?? "TAGS") return cell } else { return nil diff --git a/src/iOS/POI/POITabBarController.swift b/src/iOS/POI/POITabBarController.swift index ca4c67ef3..6d4707063 100644 --- a/src/iOS/POI/POITabBarController.swift +++ b/src/iOS/POI/POITabBarController.swift @@ -21,23 +21,16 @@ class POITabBarController: UITabBarController { keyValueDict = selection?.tags ?? [:] relationList = selection?.parentRelations ?? [] + let hideAttributesTab = Self.shouldHideAttributesTab(for: selection) var tabIndex = UserPrefs.shared.poiTabIndex.value ?? 0 - if tabIndex == 2, - selection == nil - { + if hideAttributesTab, tabIndex == 2 { tabIndex = 0 } - if selection == nil { - // don't show attributes page - var vcList = viewControllers! - vcList.removeLast() - self.viewControllers = vcList + if hideAttributesTab { + removeAttributesTabFromViewControllers() } selectedIndex = tabIndex - // hide attributes tab on new objects - updatePOIAttributesTabBarItemVisibility(withSelectedObject: selection) - if #available(iOS 17, *) { // On MacCatalyst (and maybe iPad) UITabBar is broken. // This fixes it. @@ -73,27 +66,17 @@ class POITabBarController: UITabBarController { selectedViewController?.dismiss(animated: true) } - /// Hides the POI attributes tab bar item when the user is adding a new item, since it doesn't have any attributes yet. - /// - Parameter selectedObject: The object that the user selected on the map. - func updatePOIAttributesTabBarItemVisibility(withSelectedObject selectedObject: OsmBaseObject?) { - let isAddingNewItem = selectedObject == nil - if isAddingNewItem { - // Remove the `POIAttributesViewController`. - var viewControllersToKeep: [UIViewController] = [] - for controller in viewControllers ?? [] { - if controller is UINavigationController, - (controller as? UINavigationController)?.viewControllers.first is POIAttributesViewController - { - // For new objects, the navigation controller that contains the view controller - // for POI attributes is not needed; ignore it. - return - } else { - viewControllersToKeep.append(controller) - } - } - - setViewControllers(viewControllersToKeep, animated: false) - } + /// Attributes are only useful for objects that exist on the server (positive OSM id). + private static func shouldHideAttributesTab(for selection: OsmBaseObject?) -> Bool { + guard let selection else { return true } + return selection.ident < 0 + } + + private func removeAttributesTabFromViewControllers() { + var vcList = viewControllers ?? [] + guard vcList.count > 2 else { return } + vcList.removeLast() + viewControllers = vcList } func setFeatureKey(_ key: String, value: String?) { From e1201263371266d63b7b97dbd6e821cc6daaa5e7 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sat, 30 May 2026 16:00:40 +0000 Subject: [PATCH 2/2] Add POITabBarController unit tests (merged from PR #6) Expose resolvedTabBar for tab index clamping; adopt in viewDidLoad. Supersedes duplicate Attributes-tab logic in draft PR #6. Co-Authored-By: Tobias --- src/iOS/Go Map!!.xcodeproj/project.pbxproj | 4 + .../POITabBarControllerTestCase.swift | 91 +++++++++++++++++++ src/iOS/Localizable.xcstrings | 9 ++ src/iOS/POI/POIAllTagsViewController.swift | 15 ++- src/iOS/POI/POITabBarController.swift | 27 ++++-- 5 files changed, 134 insertions(+), 12 deletions(-) create mode 100644 src/iOS/GoMapTests/POITabBarControllerTestCase.swift diff --git a/src/iOS/Go Map!!.xcodeproj/project.pbxproj b/src/iOS/Go Map!!.xcodeproj/project.pbxproj index 62c74711e..651ca9966 100644 --- a/src/iOS/Go Map!!.xcodeproj/project.pbxproj +++ b/src/iOS/Go Map!!.xcodeproj/project.pbxproj @@ -231,6 +231,7 @@ 64348CFE225E867800ADE7FB /* MeasureDirectionViewModelTestCase.swift in Sources */ = {isa = PBXBuildFile; fileRef = 64348CFA225E867800ADE7FB /* MeasureDirectionViewModelTestCase.swift */; }; 64348D01225E8D3F00ADE7FB /* OsmNode+Direction.swift in Sources */ = {isa = PBXBuildFile; fileRef = 64348D00225E8D3F00ADE7FB /* OsmNode+Direction.swift */; }; 64348D03225E8E4300ADE7FB /* OsmNode_DirectionTestCase.swift in Sources */ = {isa = PBXBuildFile; fileRef = 64348D02225E8E4300ADE7FB /* OsmNode_DirectionTestCase.swift */; }; + A5E8F2022601CEC900EC0A60 /* POITabBarControllerTestCase.swift in Sources */ = {isa = PBXBuildFile; fileRef = A5E8F2012601CEC900EC0A60 /* POITabBarControllerTestCase.swift */; }; 64348D13225EAA5D00ADE7FB /* MapViewUITestCase.swift in Sources */ = {isa = PBXBuildFile; fileRef = 64348D12225EAA5D00ADE7FB /* MapViewUITestCase.swift */; }; 6442666722540EDF00C0D545 /* Lock.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6442666422540EDF00C0D545 /* Lock.swift */; }; 6442666822540EDF00C0D545 /* Disposable.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6442666522540EDF00C0D545 /* Disposable.swift */; }; @@ -591,6 +592,7 @@ 64348CFA225E867800ADE7FB /* MeasureDirectionViewModelTestCase.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = MeasureDirectionViewModelTestCase.swift; sourceTree = ""; }; 64348D00225E8D3F00ADE7FB /* OsmNode+Direction.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "OsmNode+Direction.swift"; sourceTree = ""; }; 64348D02225E8E4300ADE7FB /* OsmNode_DirectionTestCase.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OsmNode_DirectionTestCase.swift; sourceTree = ""; }; + A5E8F2012601CEC900EC0A60 /* POITabBarControllerTestCase.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = POITabBarControllerTestCase.swift; sourceTree = ""; }; 64348D08225EA24E00ADE7FB /* GoMapUITests.xctest */ = {isa = PBXFileReference; explicitFileType = wrapper.cfbundle; includeInIndex = 0; path = GoMapUITests.xctest; sourceTree = BUILT_PRODUCTS_DIR; }; 64348D0C225EA24E00ADE7FB /* Info.plist */ = {isa = PBXFileReference; lastKnownFileType = text.plist.xml; path = Info.plist; sourceTree = ""; }; 64348D12225EAA5D00ADE7FB /* MapViewUITestCase.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MapViewUITestCase.swift; sourceTree = ""; }; @@ -1188,6 +1190,7 @@ 64348CFA225E867800ADE7FB /* MeasureDirectionViewModelTestCase.swift */, 64348CF6225E867800ADE7FB /* Mocks */, 64348D02225E8E4300ADE7FB /* OsmNode_DirectionTestCase.swift */, + A5E8F2012601CEC900EC0A60 /* POITabBarControllerTestCase.swift */, 64348CEC225E7CD900ADE7FB /* GoMapTests.swift */, 64C072FC22622B9C00598078 /* Vendor */, 64348CEE225E7CD900ADE7FB /* Info.plist */, @@ -1815,6 +1818,7 @@ 64E21EB822651F2D004605D7 /* XCTestCase+UserDefaults.swift in Sources */, 64348CFD225E867800ADE7FB /* CLHeadingMock.swift in Sources */, 64348D03225E8E4300ADE7FB /* OsmNode_DirectionTestCase.swift in Sources */, + A5E8F2022601CEC900EC0A60 /* POITabBarControllerTestCase.swift in Sources */, 64E21EB522651C06004605D7 /* OSMMapDataTestCase.swift in Sources */, ); runOnlyForDeploymentPostprocessing = 0; diff --git a/src/iOS/GoMapTests/POITabBarControllerTestCase.swift b/src/iOS/GoMapTests/POITabBarControllerTestCase.swift new file mode 100644 index 000000000..de6d57899 --- /dev/null +++ b/src/iOS/GoMapTests/POITabBarControllerTestCase.swift @@ -0,0 +1,91 @@ +// +// POITabBarControllerTestCase.swift +// GoMapTests +// + +@testable import Go_Map__ +import XCTest + +class POITabBarControllerTestCase: XCTestCase { + // MARK: shouldHideAttributesTab + + func testShouldHideAttributesTabIsTrueForNilSelection() { + XCTAssertTrue(POITabBarController.shouldHideAttributesTab(for: nil)) + } + + func testShouldHideAttributesTabIsTrueForPendingNode() { + let node = OsmNode(asUserCreated: "") + XCTAssertLessThan(node.ident, 0) + XCTAssertTrue(POITabBarController.shouldHideAttributesTab(for: node)) + } + + func testShouldHideAttributesTabIsFalseForUploadedNode() { + let node = OsmNode( + withVersion: 1, + changeset: 0, + user: "", + uid: 0, + ident: 1, + timestamp: "", + tags: [:]) + XCTAssertGreaterThan(node.ident, 0) + XCTAssertFalse(POITabBarController.shouldHideAttributesTab(for: node)) + } + + func testShouldHideAttributesTabIsTrueForPendingWay() { + let way = OsmWay(asUserCreated: "") + XCTAssertLessThan(way.ident, 0) + XCTAssertTrue(POITabBarController.shouldHideAttributesTab(for: way)) + } + + // MARK: resolvedTabBar + + func testResolvedTabBarNilSelection() { + assertResolvedTabBar(savedIndex: 0, selection: nil, expectedTabCount: 2, expectedSelectedIndex: 0) + assertResolvedTabBar(savedIndex: 1, selection: nil, expectedTabCount: 2, expectedSelectedIndex: 1) + assertResolvedTabBar(savedIndex: 2, selection: nil, expectedTabCount: 2, expectedSelectedIndex: 0) + } + + func testResolvedTabBarPendingNode() { + let node = OsmNode(asUserCreated: "") + assertResolvedTabBar(savedIndex: 0, selection: node, expectedTabCount: 2, expectedSelectedIndex: 0) + assertResolvedTabBar(savedIndex: 1, selection: node, expectedTabCount: 2, expectedSelectedIndex: 1) + assertResolvedTabBar(savedIndex: 2, selection: node, expectedTabCount: 2, expectedSelectedIndex: 0) + } + + func testResolvedTabBarUploadedNode() { + let node = OsmNode( + withVersion: 1, + changeset: 0, + user: "", + uid: 0, + ident: 42, + timestamp: "", + tags: [:]) + assertResolvedTabBar(savedIndex: 0, selection: node, expectedTabCount: 3, expectedSelectedIndex: 0) + assertResolvedTabBar(savedIndex: 1, selection: node, expectedTabCount: 3, expectedSelectedIndex: 1) + assertResolvedTabBar(savedIndex: 2, selection: node, expectedTabCount: 3, expectedSelectedIndex: 2) + } + + func testResolvedTabBarPendingWay() { + let way = OsmWay(asUserCreated: "") + assertResolvedTabBar(savedIndex: 0, selection: way, expectedTabCount: 2, expectedSelectedIndex: 0) + assertResolvedTabBar(savedIndex: 1, selection: way, expectedTabCount: 2, expectedSelectedIndex: 1) + assertResolvedTabBar(savedIndex: 2, selection: way, expectedTabCount: 2, expectedSelectedIndex: 0) + } + + // MARK: Helpers + + private func assertResolvedTabBar( + savedIndex: Int, + selection: OsmBaseObject?, + expectedTabCount: Int, + expectedSelectedIndex: Int, + file: StaticString = #file, + line: UInt = #line + ) { + let result = POITabBarController.resolvedTabBar(savedIndex: savedIndex, selection: selection) + XCTAssertEqual(result.tabCount, expectedTabCount, file: file, line: line) + XCTAssertEqual(result.selectedIndex, expectedSelectedIndex, file: file, line: line) + } +} diff --git a/src/iOS/Localizable.xcstrings b/src/iOS/Localizable.xcstrings index df867b596..467edb047 100644 --- a/src/iOS/Localizable.xcstrings +++ b/src/iOS/Localizable.xcstrings @@ -25452,6 +25452,9 @@ } } } + }, + "Change preset" : { + }, "Changeset" : { "comment" : "OSM changeset identifier", @@ -46840,6 +46843,9 @@ } } } + }, + "Opens the preset picker" : { + }, "OSM Data" : { "comment" : "Delete cached data", @@ -54467,6 +54473,9 @@ } } } + }, + "Shows Common Tags" : { + }, "Something went wrong while attempting to restore your data. Any pending changes have been lost. Sorry." : { "localizations" : { diff --git a/src/iOS/POI/POIAllTagsViewController.swift b/src/iOS/POI/POIAllTagsViewController.swift index 1140ab84a..a5932cb5e 100644 --- a/src/iOS/POI/POIAllTagsViewController.swift +++ b/src/iOS/POI/POIAllTagsViewController.swift @@ -90,10 +90,17 @@ private class SectionHeaderCell: UITableViewHeaderFooterView { } @objc func showCommonTagsTab(_ sender: Any?) { - guard let tabBar = tabBarController as? POITabBarController else { return } - UserPrefs.shared.poiTabIndex.value = 0 - guard tabBar.selectedIndex != 0 else { return } - tabBar.slideTabTo(tabIndex: 0) + var r: UIResponder = self + while true { + if let tabBar = r as? POITabBarController { + UserPrefs.shared.poiTabIndex.value = 0 + guard tabBar.selectedIndex != 0 else { return } + tabBar.slideTabTo(tabIndex: 0) + return + } + guard let next = r.next else { return } + r = next + } } @objc func pickFeature(_ sender: Any?) { diff --git a/src/iOS/POI/POITabBarController.swift b/src/iOS/POI/POITabBarController.swift index 6d4707063..f2ac35347 100644 --- a/src/iOS/POI/POITabBarController.swift +++ b/src/iOS/POI/POITabBarController.swift @@ -21,15 +21,12 @@ class POITabBarController: UITabBarController { keyValueDict = selection?.tags ?? [:] relationList = selection?.parentRelations ?? [] - let hideAttributesTab = Self.shouldHideAttributesTab(for: selection) - var tabIndex = UserPrefs.shared.poiTabIndex.value ?? 0 - if hideAttributesTab, tabIndex == 2 { - tabIndex = 0 - } - if hideAttributesTab { + let savedIndex = UserPrefs.shared.poiTabIndex.value ?? 0 + let resolved = Self.resolvedTabBar(savedIndex: savedIndex, selection: selection) + if resolved.tabCount == 2 { removeAttributesTabFromViewControllers() } - selectedIndex = tabIndex + selectedIndex = resolved.selectedIndex if #available(iOS 17, *) { // On MacCatalyst (and maybe iPad) UITabBar is broken. @@ -67,11 +64,25 @@ class POITabBarController: UITabBarController { } /// Attributes are only useful for objects that exist on the server (positive OSM id). - private static func shouldHideAttributesTab(for selection: OsmBaseObject?) -> Bool { + static func shouldHideAttributesTab(for selection: OsmBaseObject?) -> Bool { guard let selection else { return true } return selection.ident < 0 } + /// Tab order: 0 Common Tags, 1 All Tags, 2 Attributes. + static func resolvedTabBar( + savedIndex: Int, + selection: OsmBaseObject? + ) -> (tabCount: Int, selectedIndex: Int) { + let hideAttributes = shouldHideAttributesTab(for: selection) + let tabCount = hideAttributes ? 2 : 3 + var selectedIndex = savedIndex + if hideAttributes, savedIndex == 2 { + selectedIndex = 0 + } + return (tabCount, selectedIndex) + } + private func removeAttributesTabFromViewControllers() { var vcList = viewControllers ?? [] guard vcList.count > 2 else { return }