Skip to content
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

Playback 2024: Loading and Error screens #2369

Merged
merged 5 commits into from
Oct 31, 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
4 changes: 4 additions & 0 deletions PocketCastsTests/Tests/End of Year/StoriesModelTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,8 @@ class StoriesModelTests: XCTestCase {
class MockStoriesDataSource: StoriesDataSource {
var indicatorColor: Color = .white

var primaryBackgroundColor: Color = .black

var numberOfStories: Int = 2

var didCallStoryForWithStoryNumber: Int?
Expand Down Expand Up @@ -196,6 +198,8 @@ class MockStoriesDataSource: StoriesDataSource {
class MockStoriesWithPlusDataSource: StoriesDataSource {
var indicatorColor: Color = .white

var primaryBackgroundColor: Color = .black

var numberOfStories: Int = 4

var didCallStoryForWithStoryNumber: Int?
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ class EndOfYear2023StoriesModel: StoryModel {
.white
}

var primaryBackgroundColor: Color {
.black
}

required init() {}

func populate(with dataManager: DataManager) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ class EndOfYear2024StoriesModel: StoryModel {
.black
}

var primaryBackgroundColor: Color {
Color(hex: "EE661C")
}

required init() { }

func populate(with dataManager: DataManager) {
Expand Down
1 change: 1 addition & 0 deletions podcasts/End of Year/EndOfYearStoriesBuilder.swift
Original file line number Diff line number Diff line change
Expand Up @@ -60,4 +60,5 @@ protocol StoryModel {
/// Shown at the bottom of the story as an additional safe area
func footerShareView() -> AnyView?
var indicatorColor: Color { get }
var primaryBackgroundColor: Color { get }
}
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ struct Top5Podcasts2024Story: ShareableStory {
.disabled(!isSmallScreen) // Disable scrolling on larger where we shouldn't be clipping.
.frame(height: geometry.size.height * 0.65)

Text("And you were big on these shows too!")
Text(L10n.eoyStoryTopPodcastsTitle)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bjtitus I found this text unlocalized. I changed it but let me know if I need to revert it.

.font(.system(size: 30, weight: .bold))
}
.padding()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,10 @@ class EndOfYearStoriesDataSource: StoriesDataSource {
var indicatorColor: Color {
model.indicatorColor
}

var primaryBackgroundColor: Color {
model.primaryBackgroundColor
}
}

extension Array where Element: CaseIterable & Equatable {
Expand Down
4 changes: 4 additions & 0 deletions podcasts/End of Year/StoriesDataSource.swift
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,11 @@ protocol StoriesDataSource {
/// Shown at the bottom of the story as an additional safe area
func footerShareView() -> AnyView?

/// Color of the top Story progress indicator
var indicatorColor: Color { get }

/// Color of the primary background
var primaryBackgroundColor: Color { get }
}

extension StoriesDataSource {
Expand Down
6 changes: 5 additions & 1 deletion podcasts/End of Year/StoriesModel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ class StoriesModel: ObservableObject {

func start() {
cancellable = publisher.autoconnect().sink(receiveValue: { _ in
guard self.numberOfStories > 0 else {
guard self.currentStory != nil, self.numberOfStories > 0 else {
return
}

Expand Down Expand Up @@ -250,6 +250,10 @@ class StoriesModel: ObservableObject {
var indicatorColor: Color {
dataSource.indicatorColor
}

var primaryBackgroundColor: Color {
dataSource.primaryBackgroundColor
}
}

private extension StoriesModel {
Expand Down
8 changes: 5 additions & 3 deletions podcasts/End of Year/Views/StoriesView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -98,28 +98,30 @@ struct StoriesView: View {

VStack(spacing: 15) {
let progress = syncProgressModel.progress
CircularProgressView(value: progress, stroke: Color.white, strokeWidth: 6)
CircularProgressView(value: progress, stroke: model.indicatorColor, strokeWidth: 6)
.frame(width: 40, height: 40)
Text(L10n.loading)
.foregroundColor(.white)
.foregroundColor(model.indicatorColor)
.font(style: .body)
}

storySwitcher
Copy link
Contributor Author

@danielebogo danielebogo Oct 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bjtitus I'm not sure if this needs to be here. Forcing the loading view or error to be in the body causes a crash when tapping. the start method called causes an Inf value calculating the current progress. WDYT?

header
}
.background(model.primaryBackgroundColor)
}

var failed: some View {
ZStack {
Spacer()

Text(L10n.eoyStoriesFailed)
.foregroundColor(.white)
.foregroundColor(model.indicatorColor)

storySwitcher
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bjtitus Same here. WDYT?

header
}
.background(model.primaryBackgroundColor)
.onAppear {
Analytics.track(.endOfYearStoriesFailedToLoad)
}
Expand Down