Conversation
tmsm1999
changed the title
Shared/Styles/Fonts.swift
Outdated
| static let eventTitleFont: Self = Font.system(.title3, weight: .bold) | ||
| static let eventRoundNumberFont: Self = Font.body.italic() | ||
| static let liveSessionTitleFont: Self = Font.body.bold() | ||
| static let chevronRightFont: Self = Font.largeTitle | ||
| static let liveTextFont: Self = Font.body.bold() |
There was a problem hiding this comment.
You don't need Font. you can simply have the value 💅
|
|
||
| protocol EventStatusBackgroundStylerRepresentable { | ||
|
|
||
| var eventStatus: GrandPrixCardViewModel.EventStatus { get } |
There was a problem hiding this comment.
Not really a fan of having a Representable consuming another viewModel type 😕, more, in this particular case a Styler should be fully independent to be able to be used everywhere you want, so you have 2 solutions here:
1- Maybe this should be part of the Sessions Model or any other Model that makes sense
2 - Put this inside the EventStatusBackgroundStyler
| let id: String // Driver Ticker | ||
| let value: Value | ||
|
|
||
| enum Value: Comparable { |
There was a problem hiding this comment.
This can conform to Int, like that it would be easier to instantiate it when having the results from the API
| enum Value: Comparable { | |
| enum Value: Int, Comparable { | |
| case first = 1 | |
| case second = 2 | |
| case third = 3 | |
| .... | |
| } | |
| .init(driverTicker: "ALO", value: .init(rawValue: sessionResult.position)) // postion = 1,2,3... |
There was a problem hiding this comment.
I created a new property called driverTicker and now the id property is instantiated to UUID().uuidString. We need to review this together to make sure everything is ok.
There was a problem hiding this comment.
ok, whenever you want 🙂
| var currentSessionTitle: String? { get } | ||
| var currentSessionDetails: String? { get } | ||
|
|
||
| var drivers: [DriverResult]? { get } |
There was a problem hiding this comment.
should we remove the optional from here? 🤔
| init( | ||
| round: Int, | ||
| title: String, | ||
| grandPrixDate: String? = .none, |
| case .live, .current: | ||
| if let title = viewModel.currentSessionTitle, let details = viewModel.currentSessionDetails { | ||
| sessionDetailsComponent(title: title, details: details) | ||
| } | ||
| case .upcoming: | ||
| if let details = viewModel.currentSessionDetails { | ||
| sessionDetailsComponent(details: details) | ||
| } |
There was a problem hiding this comment.
| case .live, .current: | |
| if let title = viewModel.currentSessionTitle, let details = viewModel.currentSessionDetails { | |
| sessionDetailsComponent(title: title, details: details) | |
| } | |
| case .upcoming: | |
| if let details = viewModel.currentSessionDetails { | |
| sessionDetailsComponent(details: details) | |
| } | |
| case .live, .current, .upcoming: | |
| if let details = viewModel.currentSessionDetails { | |
| sessionDetailsComponent(title: viewModel.eventStatus == .upcoming ? nil : viewModel.currentSessionTitle, details: details) | |
| } |
Another way simpler to do this is you probably should fire the title and the details from the enum itself, meaning the enum should be something like
enum Status {
case live(String, Details)
case current(String, Details)
case upcoming(Details)
case finished([DriverResult])
}
and like this you don't need half of the properties you have inside the VM and you simply use the enum to build your views.
What do you think?
| .font(.liveSessionTitleFont) | ||
| } | ||
|
|
||
| if viewModel.eventStatus == .live || viewModel.eventStatus == .upcoming { |
There was a problem hiding this comment.
I might be getting this wrong but this info should come from the main view that uses the function, the function should not have this kind of logic, at least related to the Status that is something already handled in the main view
|
Great work as always keep pushing 👏👏👏 |
- Correction to UI.
2 participants
No description provided.