diff --git a/CHANGELOG.md b/CHANGELOG.md index 83319ff..3217038 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,13 @@ open: **a second build under the same version goes under the already cut heading, not back under `Unreleased`.** Date the heading and add its compare link once the version tag exists. +## [Unreleased] + +### Fixed + +- A long document name no longer pushes the buttons off the bar. The buttons + stay in place, and the name is shortened in the middle. + ## [1.46] 1.45 did not go out in the store, so the store notes of 1.46 carry its changes diff --git a/OpenDocumentReader/DocumentTitleLabel.swift b/OpenDocumentReader/DocumentTitleLabel.swift index 4d86e02..4c17d6a 100644 --- a/OpenDocumentReader/DocumentTitleLabel.swift +++ b/OpenDocumentReader/DocumentTitleLabel.swift @@ -1,26 +1,11 @@ import UIKit /// The document's name as the tool bar shows it. -/// -/// A bar item is as wide as what it holds, so a long name would push the -/// buttons off the end. This one truncates instead. final class DocumentTitleLabel: UILabel { - /// The most the name may take. - var maximumWidth: CGFloat = .greatestFiniteMagnitude { - didSet { - guard maximumWidth != oldValue else { return } - - invalidateIntrinsicContentSize() - } - } - override init(frame: CGRect) { super.init(frame: frame) - // the bar sizes it from its intrinsic width, which is what caps it - translatesAutoresizingMaskIntoConstraints = false - // the middle of a name says more than its end: "Q3 report (final)" and // "Q3 report (draft)" differ where a tail truncation cuts lineBreakMode = .byTruncatingMiddle @@ -33,16 +18,4 @@ final class DocumentTitleLabel: UILabel { required init?(coder: NSCoder) { fatalError("init(coder:) has not been implemented") } - - override var intrinsicContentSize: CGSize { - capped(super.intrinsicContentSize) - } - - override func sizeThatFits(_ size: CGSize) -> CGSize { - capped(super.sizeThatFits(size)) - } - - private func capped(_ size: CGSize) -> CGSize { - CGSize(width: min(size.width, max(0, maximumWidth)), height: size.height) - } } diff --git a/OpenDocumentReader/DocumentViewController.swift b/OpenDocumentReader/DocumentViewController.swift index f306bfa..e7d21d8 100644 --- a/OpenDocumentReader/DocumentViewController.swift +++ b/OpenDocumentReader/DocumentViewController.swift @@ -59,9 +59,6 @@ class DocumentViewController: UIViewController, DocumentDelegate, UISearchBarDel /// The document's name, sitting in the bar's empty middle. let documentTitleLabel = DocumentTitleLabel() - private lazy var documentTitleItem = UIBarButtonItem(customView: documentTitleLabel) - private lazy var documentTitleSpacer = UIBarButtonItem( - barButtonSystemItem: .flexibleSpace, target: nil, action: nil) /// The bar as the storyboard has it, taken before anything is removed, since /// that is the only moment every button is there to be read. @@ -69,7 +66,7 @@ class DocumentViewController: UIViewController, DocumentDelegate, UISearchBarDel /// Whether the document on screen can be edited and searched. Neither button /// stays in the bar when it cannot be used. - private var canEdit = false { didSet { updateToolBar() } } + var canEdit = false { didSet { updateToolBar() } } /// Whether the document is a pdf that takes marks. private var canMark = false { didSet { @@ -147,7 +144,7 @@ class DocumentViewController: UIViewController, DocumentDelegate, UISearchBarDel /// Set by a save, so the page that loads next is put back into the mode. private var resumesEditAfterLoad = false - private var canSearch = false { + var canSearch = false { didSet { updateToolBar() @@ -1019,16 +1016,10 @@ class DocumentViewController: UIViewController, DocumentDelegate, UISearchBarDel at: pen + 1) } - /// A gap either side of the name, which is what puts it in the middle. + /// On the bar rather than in it: a bar item pushes the buttons aside, and + /// the name has to give way to them instead. private func setUpDocumentTitle() { - // a glass capsule is what a button looks like, and this is not one - if #available(iOS 26.0, *) { - documentTitleItem.hidesSharedBackground = true - } - - guard let back = toolBarItems.firstIndex(where: { $0 === barButtonItem }) else { return } - - toolBarItems.insert(contentsOf: [documentTitleSpacer, documentTitleItem], at: back + 1) + toolBar.addSubview(documentTitleLabel) updateDocumentTitle() } @@ -1041,19 +1032,19 @@ class DocumentViewController: UIViewController, DocumentDelegate, UISearchBarDel override func viewDidLayoutSubviews() { super.viewDidLayoutSubviews() - updateDocumentTitleWidth() + layOutDocumentTitle() } - /// What the bar has left once its buttons have taken theirs. - /// - /// Measured against the view rather than the bar itself: on the first pass - /// the bar still carries the width the storyboard drew it at, and the name - /// keeps whatever width it is first measured at. - private func updateDocumentTitleWidth() { - let buttons = (toolBar.items ?? []).filter { $0.customView == nil && $0.image != nil } + /// The name takes what the buttons leave between the back button and the + /// rest. The bar does not say where it put a button, so the room is + /// counted from how many there are. + private func layOutDocumentTitle() { + let buttons = (toolBar.items ?? []).filter { $0.image != nil } + let start = Self.toolBarButtonWidth + Self.toolBarTitleGap + let end = toolBar.bounds.width - CGFloat(buttons.count - 1) * Self.toolBarButtonWidth - Self.toolBarTitleGap - documentTitleLabel.maximumWidth = - view.bounds.width - CGFloat(buttons.count) * Self.toolBarButtonWidth - Self.toolBarTitleGap + documentTitleLabel.frame = CGRect( + x: start, y: 0, width: max(0, end - start), height: toolBar.bounds.height) } /// What one button takes of the bar. From iOS 26 a glass capsule with air @@ -1073,13 +1064,13 @@ class DocumentViewController: UIViewController, DocumentDelegate, UISearchBarDel /// and the magnifier and the name stand down - six buttons and a name do /// not fit a phone's bar. A pdf mark is never put back, so redo stays out. private func updateToolBar() { + documentTitleLabel.isHidden = isEditingDocument + view.setNeedsLayout() + toolBar.items = toolBarItems.filter { item in if item === editButton || item === editButtonSpacer { return canEdit } - if item === documentTitleItem || item === documentTitleSpacer { - return !isEditingDocument - } if item === redoButton || item === redoButtonSpacer { return canEdit && isEditingDocument && !canMark } diff --git a/OpenDocumentReaderTests/DocumentTitleTests.swift b/OpenDocumentReaderTests/DocumentTitleTests.swift index 353d555..26cd548 100644 --- a/OpenDocumentReaderTests/DocumentTitleTests.swift +++ b/OpenDocumentReaderTests/DocumentTitleTests.swift @@ -18,7 +18,7 @@ class DocumentTitleTests: XCTestCase { /// The file need not exist: the name is read from the URL, and the bar shows /// it before the document is opened. - private func present(_ name: String) throws { + private func present(_ name: String, width: CGFloat = 390) throws { let storyboard = UIStoryboard(name: "Main", bundle: Bundle(for: DocumentViewController.self)) controller = try XCTUnwrap( storyboard.instantiateViewController(withIdentifier: "TextDocumentViewController") @@ -28,7 +28,7 @@ class DocumentTitleTests: XCTestCase { for: .documentDirectory, in: .userDomainMask, appropriateFor: nil, create: false) controller.document = Document(fileURL: documents.appendingPathComponent(name)) - window = UIWindow(frame: CGRect(x: 0, y: 0, width: 390, height: 844)) + window = UIWindow(frame: CGRect(x: 0, y: 0, width: width, height: 844)) window.rootViewController = controller window.makeKeyAndVisible() @@ -39,7 +39,7 @@ class DocumentTitleTests: XCTestCase { try present("Quarterly report.odt") XCTAssertEqual(controller.documentTitleLabel.text, "Quarterly report") - XCTAssertTrue((controller.toolBar.items ?? []).contains { $0.customView === controller.documentTitleLabel }) + XCTAssertTrue(controller.documentTitleLabel.isDescendant(of: controller.toolBar)) } /// The name is what a reader recognises the file by, so a dot in it is part @@ -50,52 +50,65 @@ class DocumentTitleTests: XCTestCase { XCTAssertEqual(controller.documentTitleLabel.text, "Minutes 12.03") } - func testTheNameSitsBetweenTheBackButtonAndTheRest() throws { - try present("Quarterly report.odt") - - let items = try XCTUnwrap(controller.toolBar.items) - let name = try XCTUnwrap(items.firstIndex { $0.customView === controller.documentTitleLabel }) - let back = try XCTUnwrap(items.firstIndex { $0 === controller.barButtonItem }) - let menu = try XCTUnwrap(items.firstIndex { $0 === controller.menuButton }) + /// Every button a document can have, as a bar that can edit and search + /// shows them. + private func showEveryButton() { + controller.canEdit = true + controller.canSearch = true + controller.view.layoutIfNeeded() + } - XCTAssertTrue(back < name && name < menu) + /// Where a button is drawn, read through a private key: the bar has no + /// public way to say. The iOS 26 bar does not draw a button put back into it + /// while the window is a test's, so there the test cannot look. + private func frame(of item: UIBarButtonItem) throws -> CGRect { + guard let view = item.value(forKey: "view") as? UIView, view.window != nil, view.bounds.width > 0 else { + throw XCTSkip("the bar did not draw this button") + } + return view.convert(view.bounds, to: controller.toolBar) } - /// What used to be the risk: a name long enough to push the buttons off the - /// end of the bar. - func testALongNameIsTruncatedRatherThanWidening() throws { - try present("Quarterly report for the whole board, final revision.odt") + /// The bug this guards: a long name pushed the buttons off the end of the + /// bar. + func testALongNameLeavesTheButtonsInPlace() throws { + try present("Quarterly report.odt", width: 375) + let short = (try frame(of: controller.barButtonItem), try frame(of: controller.menuButton)) + window.isHidden = true - let label = controller.documentTitleLabel + try present("Quarterly report for the whole board, final revision, with appendices.odt", width: 375) + let long = (try frame(of: controller.barButtonItem), try frame(of: controller.menuButton)) - XCTAssertLessThanOrEqual(label.bounds.width, label.maximumWidth) - XCTAssertLessThan(label.maximumWidth, label.text!.size(withAttributes: [.font: label.font!]).width) + XCTAssertEqual(short.0, long.0) + XCTAssertEqual(short.1, long.1) } - /// The bar is the same width whoever is in it, so a name has less room when - /// there are more buttons to leave room for. - func testFewerButtonsLeaveTheNameMoreRoom() throws { - try present("Quarterly report.odt") + func testALongNameIsCutShortBetweenTheButtons() throws { + for width: CGFloat in [375, 390, 402] { + try present("Quarterly report for the whole board, final revision, with appendices.odt", width: width) + showEveryButton() - let withEverything = controller.documentTitleLabel.maximumWidth + let label = controller.documentTitleLabel + let name = label.frame - controller.toolBar.items = (controller.toolBar.items ?? []).filter { $0 !== controller.menuButton } - controller.view.setNeedsLayout() - controller.view.layoutIfNeeded() + XCTAssertLessThan(name.width, label.text!.size(withAttributes: [.font: label.font!]).width) + for item in controller.toolBar.items ?? [] where item.image != nil { + let button = try frame(of: item) + XCTAssertFalse(name.intersects(button), "\(width)") + } - XCTAssertGreaterThan(controller.documentTitleLabel.maximumWidth, withEverything) + window.isHidden = true + } } - func testTheLabelStopsGrowingAtItsMaximum() { - let label = DocumentTitleLabel() - label.text = String(repeating: "long name ", count: 20) + /// The bar is the same width whoever is in it, so a name has less room when + /// there are more buttons to leave room for. + func testMoreButtonsLeaveTheNameLessRoom() throws { + try present("Quarterly report.odt") - label.maximumWidth = .greatestFiniteMagnitude - let unbounded = label.intrinsicContentSize.width + let withTwo = controller.documentTitleLabel.bounds.width - label.maximumWidth = 120 + showEveryButton() - XCTAssertGreaterThan(unbounded, 120) - XCTAssertEqual(label.intrinsicContentSize.width, 120) + XCTAssertLessThan(controller.documentTitleLabel.bounds.width, withTwo) } }