Skip to content

Commit 478311e

Browse files
GianniCarloGianniCarlo
committed
address feedback
1 parent 8bad245 commit 478311e

8 files changed

Lines changed: 42 additions & 27 deletions

File tree

‎BookPlayer.xcodeproj/project.pbxproj‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -862,7 +862,6 @@
862862
CA3B408256F8458669106CF9 /* IntegrationConnectionViewModelProtocol.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4D91D6855100BD1823B2674F /* IntegrationConnectionViewModelProtocol.swift */; };
863863
CCC7E1CC127610C3687B8A50 /* SortPreferencesResolving.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5C6BC9E7E76B9BAB7A0D88D2 /* SortPreferencesResolving.swift */; };
864864
D080B0A77D9844C3A0737170 /* IntegrationConnectionFormViewModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6FB91B577BC77F45EDB2AAAA /* IntegrationConnectionFormViewModel.swift */; };
865-
D0CACFD67C1F41602DCEECFC /* mp3_NO_toc_v24_longtitle.mp3 in Resources */ = {isa = PBXBuildFile; fileRef = 7170987E6662860F7FFFDEC5 /* mp3_NO_toc_v24_longtitle.mp3 */; };
866865
D2B95B92F8F84F2FA4B41BF6 /* BookEntity.swift in Sources */ = {isa = PBXBuildFile; fileRef = B33D03D266EF46B5B02FBF40 /* BookEntity.swift */; };
867866
D5E6F7081A2B3C4D5E6F7081 /* MediaServersView.swift in Sources */ = {isa = PBXBuildFile; fileRef = D5E6F7081A2B3C4D5E6F7080 /* MediaServersView.swift */; };
868867
D6BA8F162A4CA94800C2BD9A /* StorageRowView.swift in Sources */ = {isa = PBXBuildFile; fileRef = D6BA8F152A4CA94800C2BD9A /* StorageRowView.swift */; };
@@ -1600,7 +1599,6 @@
16001599
69343D35213A07B4000C425E /* VoiceOverServiceTest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = VoiceOverServiceTest.swift; sourceTree = "<group>"; };
16011600
6FB91B577BC77F45EDB2AAAA /* IntegrationConnectionFormViewModel.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = IntegrationConnectionFormViewModel.swift; sourceTree = "<group>"; };
16021601
700DFD61111AC10253D6EBAA /* SyncTasksStorageTests.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = SyncTasksStorageTests.swift; sourceTree = "<group>"; };
1603-
7170987E6662860F7FFFDEC5 /* mp3_NO_toc_v24_longtitle.mp3 */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = audio.mp3; path = mp3_NO_toc_v24_longtitle.mp3; sourceTree = "<group>"; };
16041602
75AADE0F4BFEC0640FC687FA /* TagsFlowLayout.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TagsFlowLayout.swift; sourceTree = "<group>"; };
16051603
7A823609A5B7EFC6D2D5D120 /* IntegrationLibraryItemProtocol.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = IntegrationLibraryItemProtocol.swift; sourceTree = "<group>"; };
16061604
7DDD933F7FDEDC1BF67EC567 /* TabEditingEnvironmentKey.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TabEditingEnvironmentKey.swift; sourceTree = "<group>"; };
@@ -1929,7 +1927,6 @@
19291927
12EDB8401F38342155CBBC91 /* mp3_NO_chapters.mp3 */,
19301928
1BA32C89144C68DD64FD0B62 /* mp3_NO_toc_v23.mp3 */,
19311929
4BFC5736E667F4BF8CBB9096 /* mp3_NO_toc_v24.mp3 */,
1932-
7170987E6662860F7FFFDEC5 /* mp3_NO_toc_v24_longtitle.mp3 */,
19331930
8D3EFD93E6D1E0366A3FD1E9 /* mp3_WITH_toc.mp3 */,
19341931
);
19351932
name = ChapterFixtures;
@@ -4178,7 +4175,6 @@
41784175
2B283D695990DA665D1661CC /* mp3_NO_chapters.mp3 in Resources */,
41794176
6C25A0E1ADBF2325CA78E8A4 /* mp3_NO_toc_v23.mp3 in Resources */,
41804177
4147E8513D57E852C965FC0B /* mp3_NO_toc_v24.mp3 in Resources */,
4181-
D0CACFD67C1F41602DCEECFC /* mp3_NO_toc_v24_longtitle.mp3 in Resources */,
41824178
6EC082E4BEF0CC2FAA42C0A7 /* mp3_WITH_toc.mp3 in Resources */,
41834179
);
41844180
runOnlyForDeploymentPostprocessing = 0;

‎BookPlayer/Generated/AutoMockable.generated.swift‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1425,6 +1425,7 @@ class PlayerManagerProtocolMock: PlayerManagerProtocol {
14251425
return reloadCurrentItemCallsCount > 0
14261426
}
14271427
var reloadCurrentItemClosure: (() -> Void)?
1428+
@MainActor
14281429
func reloadCurrentItem() {
14291430
reloadCurrentItemCallsCount += 1
14301431
reloadCurrentItemClosure?()

‎BookPlayer/Player/PlayerManager.swift‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -714,6 +714,7 @@ extension PlayerManager {
714714
jumpTo(chapter.start + 0.1, recordBookmark: false)
715715
}
716716

717+
@MainActor
717718
func reloadCurrentItem() {
718719
// Rebuild the in-memory item from storage so externally-changed data (e.g. re-parsed
719720
// chapters) takes effect. Playback position is preserved — it's persisted in Core Data and

‎BookPlayer/Player/PlayerManagerProtocol.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ public protocol PlayerManagerProtocol: AnyObject {
4242
func jumpToChapter(_ chapter: PlayableChapter)
4343
/// Rebuild `currentItem` from storage (and re-bind the chapter subscription) so externally
4444
/// changed data — e.g. re-parsed chapters — takes effect. Preserves playback position.
45-
func reloadCurrentItem()
45+
@MainActor func reloadCurrentItem()
4646
func markAsCompleted(_ flag: Bool)
4747
func setSpeed(_ newValue: Float)
4848
func setBoostVolume(_ newValue: Bool)

‎BookPlayer/Player/Views/Chapters/ChaptersView.swift‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -60,12 +60,16 @@ struct ChaptersView: View {
6060
Button {
6161
Task { await model.reloadChapters() }
6262
} label: {
63-
if model.isReloadingChapters {
64-
ProgressView()
65-
} else {
66-
Text("reload_button")
67-
.foregroundStyle(theme.linkColor)
68-
}
63+
// Keep the title laid out (just hidden) while loading so the spinner overlay doesn't
64+
// change the toolbar item's width.
65+
Text("reload_button")
66+
.foregroundStyle(theme.linkColor)
67+
.opacity(model.isReloadingChapters ? 0 : 1)
68+
.overlay {
69+
if model.isReloadingChapters {
70+
ProgressView()
71+
}
72+
}
6973
}
7074
.disabled(model.isReloadingChapters)
7175
.accessibilityLabel("reload_chapters_title")

‎BookPlayerTests/Resources/ChapterFixtures/mp3_NO_toc_v24_longtitle.mp3‎

-315 KB
Binary file not shown.

‎BookPlayerTests/ViewModels/ChaptersViewModelTests.swift‎

Lines changed: 24 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -21,21 +21,22 @@ final class ChaptersViewModelTests: XCTestCase {
2121
libraryServiceMock = LibraryServiceProtocolMock()
2222
}
2323

24-
private func makeItem(relativePath: String, isBoundBook: Bool) -> PlayableItem {
25-
PlayableItem(
24+
private func makeItem(relativePath: String, isBoundBook: Bool, chapterCount: Int = 1) -> PlayableItem {
25+
let chapters = (0..<chapterCount).map { index in
26+
PlayableChapter(
27+
title: "chapter \(index + 1)",
28+
author: "author",
29+
start: Double(index) * 10,
30+
duration: 10,
31+
relativePath: relativePath,
32+
remoteURL: nil,
33+
index: Int16(index)
34+
)
35+
}
36+
return PlayableItem(
2637
title: "title",
2738
author: "author",
28-
chapters: [
29-
PlayableChapter(
30-
title: "chapter",
31-
author: "author",
32-
start: 0,
33-
duration: 100,
34-
relativePath: relativePath,
35-
remoteURL: nil,
36-
index: 0
37-
)
38-
],
39+
chapters: chapters,
3940
currentTime: 0,
4041
duration: 100,
4142
relativePath: relativePath,
@@ -70,16 +71,22 @@ final class ChaptersViewModelTests: XCTestCase {
7071
XCTAssertFalse(makeSUT().canReloadChapters)
7172
}
7273

73-
func testReloadChapters_whenMoreFound_reloadsItemAndAlerts() async {
74+
func testReloadChapters_whenMoreFound_reloadsItemAndRefreshesList() async {
7475
createLocalFile(named: "reparse.m4b")
75-
playerManagerMock.currentItem = makeItem(relativePath: "reparse.m4b", isBoundBook: false)
76+
playerManagerMock.currentItem = makeItem(relativePath: "reparse.m4b", isBoundBook: false, chapterCount: 1)
7677
libraryServiceMock.reloadChaptersRelativePathReturnValue = 5
78+
// Simulate PlayerManager rebuilding currentItem from storage with the new chapter count.
79+
let reloadedItem = makeItem(relativePath: "reparse.m4b", isBoundBook: false, chapterCount: 5)
80+
playerManagerMock.reloadCurrentItemClosure = { [weak self] in
81+
self?.playerManagerMock.currentItem = reloadedItem
82+
}
7783
let sut = makeSUT()
7884

7985
await sut.reloadChapters()
8086

8187
XCTAssertTrue(libraryServiceMock.reloadChaptersRelativePathCalled)
8288
XCTAssertTrue(playerManagerMock.reloadCurrentItemCalled)
89+
XCTAssertEqual(sut.chapters.count, 5)
8390
XCTAssertNotNil(sut.currentAlert)
8491
XCTAssertFalse(sut.isReloadingChapters)
8592
}
@@ -94,6 +101,7 @@ final class ChaptersViewModelTests: XCTestCase {
94101
XCTAssertFalse(libraryServiceMock.reloadChaptersRelativePathCalled)
95102
XCTAssertFalse(playerManagerMock.reloadCurrentItemCalled)
96103
XCTAssertNotNil(sut.currentAlert)
104+
XCTAssertFalse(sut.isReloadingChapters)
97105
}
98106

99107
func testReloadChapters_whenNoAdditionalFound_alertsAndDoesNotReloadItem() async {
@@ -107,6 +115,7 @@ final class ChaptersViewModelTests: XCTestCase {
107115
XCTAssertTrue(libraryServiceMock.reloadChaptersRelativePathCalled)
108116
XCTAssertFalse(playerManagerMock.reloadCurrentItemCalled)
109117
XCTAssertNotNil(sut.currentAlert)
118+
XCTAssertFalse(sut.isReloadingChapters)
110119
}
111120

112121
func testReloadChapters_boundBook_isNoOp() async {

‎BookPlayerWatch/LocalPlayback/Player/PlayerManager.swift‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -667,6 +667,7 @@ extension PlayerManager {
667667
jumpTo(chapter.start + 0.1, recordBookmark: false)
668668
}
669669

670+
@MainActor
670671
func reloadCurrentItem() {
671672
// Rebuild `currentItem` from storage and re-bind the chapter subscription so externally
672673
// changed data takes effect while keeping the end-of-chapter handling working.
@@ -678,7 +679,10 @@ extension PlayerManager {
678679
currentItem = updatedItem
679680

680681
playableChapterSubscription?.cancel()
681-
playableChapterSubscription = updatedItem.$currentChapter.sink { [weak self] chapter in
682+
// `dropFirst()` skips the immediate replay of the current value on rebind — otherwise it
683+
// would spuriously re-fire `.chapterChange` / now-playing / widget reload for the chapter
684+
// we're already on.
685+
playableChapterSubscription = updatedItem.$currentChapter.dropFirst().sink { [weak self] chapter in
682686
guard let chapter = chapter else { return }
683687

684688
self?.setNowPlayingBookTitle(chapter: chapter)

0 commit comments

Comments
 (0)