-
Notifications
You must be signed in to change notification settings - Fork 134
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
EOY 2024: Fix header and duration issues #2393
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes work perfectly but we need to keep a retro compatibility with the EOY 23.
Can we base the changes on the current year?
podcasts/End of Year/EndOfYear.swift
Outdated
@@ -206,9 +206,13 @@ struct EndOfYear { | |||
} | |||
} | |||
|
|||
extension EndOfYear { | |||
static var defaultDuration = 10.seconds |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As we still support the 2023 version, can we switch the duration based on the currentYear
?
static var defaultDuration: TimeInterval {
switch currentYear {
case .y2024: 10.seconds
default: 7.seconds
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done here: 51402ee
podcasts/End of Year/EndOfYear.swift
Outdated
class StoriesHostingController<ContentView: View>: UIHostingController<ContentView> { | ||
override var preferredStatusBarStyle: UIStatusBarStyle { | ||
.lightContent | ||
.darkContent |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here, can we switch on the current year like above?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done here: 51402ee
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for addressing those comments!
| 📘 Part of: #2250 |
|:---:|
Fixes #2385 #2386
This PR fixes three issues on the EOY 2024:
To test
Checklist
CHANGELOG.md
if necessary.