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

Check favorite sessions in Schedule View #41

Open
wants to merge 25 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
875b195
add star icon in the right of session row
ewa1989 Mar 17, 2024
33888d9
change icon according to the session is favorited
ewa1989 Mar 17, 2024
08a2b0d
create new action, view favoriteIconTapped
ewa1989 Mar 17, 2024
c32c5bf
change to toggle favorite condition of tapped session
ewa1989 Mar 17, 2024
cf49338
change to save and load conferences in UserDefaullts
ewa1989 Mar 17, 2024
5a32df4
Merge commit '1b5005da964abcaf5a5865b68347ab32d6e72aa2' into add-chec…
ewa1989 Mar 18, 2024
846098f
change save data type as `[String: [Session]]` for performance
ewa1989 Mar 20, 2024
23983b5
extract favorites load/save methods to FileClient, separating with fe…
ewa1989 Mar 20, 2024
a7a611b
Merge branch 'main' into add-check-favorite-session-feature
ewa1989 Mar 20, 2024
0e8b403
change file save/load area to Document that is more appropriate becau…
ewa1989 Mar 20, 2024
daa7fa7
fix failed test due to adding load Favorites when onAppear
ewa1989 Mar 20, 2024
1a5419a
separate saving favorites and updating state for testability
ewa1989 Mar 20, 2024
35b4d14
add tests for adding/removing favorites
ewa1989 Mar 20, 2024
f2622b8
extract json encoder and decorder as JSONHandler
ewa1989 Mar 20, 2024
b648df2
Revert "extract json encoder and decorder as JSONHandler"
ewa1989 Mar 21, 2024
a929586
separate FileClient
ewa1989 Mar 21, 2024
4e279e4
move mocks that are intended for specific behavior near related tests
ewa1989 Mar 21, 2024
1db51b5
Merge branch 'main' into add-check-favorite-session-feature
ewa1989 Mar 21, 2024
d3a49ab
remove unused mock
ewa1989 Mar 31, 2024
04cefc0
fix to separate load favorites from fetch data, and to assert separat…
ewa1989 Mar 31, 2024
d3b9901
to keep models having no logics, move logics in Favorites to Reducer …
ewa1989 Mar 31, 2024
cfb2fa3
rename saveDataToFile and loadDataFromFile to explain exactly how the…
ewa1989 Mar 31, 2024
77b7a09
change variable name of Conference, day to conference, as other place…
ewa1989 Apr 1, 2024
0e3b9f8
fix to combine load response into fetch response for simplicity.
ewa1989 Apr 11, 2024
6cca9bd
move a part of logic to Reducer for collecting logics relative with s…
ewa1989 Apr 11, 2024
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
43 changes: 40 additions & 3 deletions MyLibrary/Sources/DataClient/Client.swift
Original file line number Diff line number Diff line change
Expand Up @@ -10,23 +10,29 @@ public struct DataClient {
public var fetchWorkshop: @Sendable () throws -> Conference
public var fetchSponsors: @Sendable () throws -> Sponsors
public var fetchOrganizers: @Sendable () throws -> [Organizer]
public var saveDay1: @Sendable (Conference) throws -> Void
public var saveDay2: @Sendable (Conference) throws -> Void
public var saveWorkshop: @Sendable (Conference) throws -> Void
Copy link
Member

Choose a reason for hiding this comment

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

It's better to create UserDefaultsClient instead of DataClient since this will be changed to ApiClient for example.
Also, favorite data is better to store in Document folder instead of User Default. What do you think?

Copy link
Author

Choose a reason for hiding this comment

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

Also, favorite data is better to store in Document folder instead of User Default. What do you think?

I agree for reading UserDefaults documentation saying

The defaults system allows an app to customize its behavior to match a user’s preferences.

Because favorite data are for saving app's state resulting in user operations, rather than customizing app's behavior.
So I will create FileClient or something... I'm thinking appropriate name that explains what it does, not how it does.
If you have any idea, please tell me.

Copy link
Author

@ewa1989 ewa1989 Mar 20, 2024

Choose a reason for hiding this comment

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

@d-date I created FileClient in 23983b5 and changed save/load area to Document in 0e8b403.
I'll change failed test and create new test for star icon behavior from now.

}

extension DataClient: DependencyKey {
private static let day1 = "day1"
private static let day2 = "day2"
private static let workshop = "workshop"

static public var liveValue: DataClient = .init(
fetchDay1: {
let data = loadDataFromBundle(fileName: "day1")
let data = loadData(dayOf: day1)
let response = try jsonDecoder.decode(Conference.self, from: data)
return response
},
fetchDay2: {
let data = loadDataFromBundle(fileName: "day2")
let data = loadData(dayOf: day2)
let response = try jsonDecoder.decode(Conference.self, from: data)
return response
},
fetchWorkshop: {
let data = loadDataFromBundle(fileName: "workshop")
let data = loadData(dayOf: workshop)
let response = try jsonDecoder.decode(Conference.self, from: data)
return response
},
Expand All @@ -39,19 +45,50 @@ extension DataClient: DependencyKey {
let data = loadDataFromBundle(fileName: "organizers")
let response = try jsonDecoder.decode([Organizer].self, from: data)
return response
},
saveDay1: { conference in
saveDataToUserDefaults(conference, as: day1)
},
saveDay2: { conference in
saveDataToUserDefaults(conference, as: day2)
},
saveWorkshop: { conference in
saveDataToUserDefaults(conference, as: workshop)
}
)

static func loadData(dayOf day: String) -> Data {
if let saveData = loadDataFromUserDefaults(key: day) {
return saveData
}
return loadDataFromBundle(fileName: day)
}

static func loadDataFromBundle(fileName: String) -> Data {
let filePath = Bundle.module.path(forResource: fileName, ofType: "json")!
let fileURL = URL(fileURLWithPath: filePath)
let data = try! Data(contentsOf: fileURL)
return data
}

static func saveDataToUserDefaults(_ conference : Conference, as key: String) {
let data = try? jsonEncoder.encode(conference)
UserDefaults.standard.set(data, forKey: key)
}

static func loadDataFromUserDefaults(key: String) -> Data? {
return UserDefaults.standard.data(forKey: key)
}
}

let jsonDecoder = {
$0.dateDecodingStrategy = .iso8601
$0.keyDecodingStrategy = .convertFromSnakeCase
return $0
}(JSONDecoder())

let jsonEncoder = {
$0.dateEncodingStrategy = .iso8601
$0.keyEncodingStrategy = .convertToSnakeCase
return $0
}(JSONEncoder())
40 changes: 40 additions & 0 deletions MyLibrary/Sources/ScheduleFeature/Schedule.swift
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ public struct Schedule {
case onAppear
case disclosureTapped(Session)
case mapItemTapped
case favoriteIconTapped(Session)
}
}

Expand Down Expand Up @@ -104,6 +105,23 @@ public struct Schedule {
#elseif os(visionOS)
return .run { _ in await openURL(url) }
#endif
case let .view(.favoriteIconTapped(session)):
switch state.selectedDay {
case .day1:
state.day1 = update(state.day1!, togglingFavoriteOf: session)
Copy link
Member

Choose a reason for hiding this comment

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

I think saving Conference object itself is not good approach. This leads such a whole updating object.
Instead, It might be better for creating [Days: Favorite], which having session hash (I know session.id is better but I don't believe id in handmade-JSON 😶‍🌫️ ).

Copy link
Author

Choose a reason for hiding this comment

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

Instead, It might be better for creating [Days: Favorite], which having session hash

Thank you.
I've chosen easier way came up with, but will try better approach of performance.

Copy link
Author

Choose a reason for hiding this comment

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

To create save method for [Days: Favorite], Days must be shown from DataClient module.
However Days is a type of LocalizedStringKey that is a frozen struct of SwiftUI.
Therefore I think it might be better to create save method for [Session: Bool], what do you think?

Further background...

  1. If putting Days in SharedModels and save [Days: [Sesson]]
    • SharedModels depends on SwiftUI, I think SharedModels wants to depends only on Foundation.
  2. If putting Days still in ScheduleFeature and make it public
    • DataClient uses Days for determining save method, and of course ScheduleFeature depends on DataClient already, so there's circular dependence.

Therefore, I resulted in it's better to create [Session: Bool] as Favorites in SharedModels, what do you think of this design idea?

Copy link
Author

Choose a reason for hiding this comment

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

Sorry, I realized that it's better to use Conference as key instead of enum Days!

Copy link
Author

Choose a reason for hiding this comment

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

@d-date In 846098f , changed save data type to [String: [Session]] which key is enforced to get from Conference.title.

case .day2:
state.day2 = update(state.day2!, togglingFavoriteOf: session)
case .day3:
state.workshop = update(state.workshop!, togglingFavoriteOf: session)
}
let day1 = state.day1!
let day2 = state.day2!
let workshop = state.workshop!
return .run { _ in
try? dataClient.saveDay1(day1)
try? dataClient.saveDay2(day2)
try? dataClient.saveWorkshop(workshop)
}
case let .fetchResponse(.success(response)):
state.day1 = response.day1
state.day2 = response.day2
Expand All @@ -122,6 +140,12 @@ public struct Schedule {
.forEach(\.path, action: \.path)
.ifLet(\.$destination, action: \.destination)
}

private func update(_ conference: Conference, togglingFavoriteOf session: Session) -> Conference {
var newValue = conference
newValue.toggleFavorite(of: session)
return newValue
}
}

@ViewAction(for: Schedule.self)
Expand Down Expand Up @@ -288,6 +312,22 @@ public struct ScheduleView: View {
}
}
.frame(maxWidth: .infinity, alignment: .leading)

favoriteIcon(for: session)
.onTapGesture {
send(.favoriteIconTapped(session))
}
}
}

@ViewBuilder
func favoriteIcon(for session: Session) -> some View {
Copy link
Member

Choose a reason for hiding this comment

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

These logic should put in Reducer so that we can write test easily.

Copy link
Author

Choose a reason for hiding this comment

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

I'm sorry that I can't image which part of logic to put in Reducer.
Does it mean create new Schedule.Action case and handle it in Reduce?
Or create computed property getting Conference in Schedule.State?

Now I implemented Favorites to use Conference.title as key for getting value of favorited [Session].

Copy link
Author

Choose a reason for hiding this comment

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

I moved these logic to Schedule.swift in d3b9901.

Copy link
Member

Choose a reason for hiding this comment

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

Please remove this method if you have moved.

Copy link
Author

Choose a reason for hiding this comment

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

Sorry for my lack of explanation.
I moved a part of logic as far as possible, so now State.favorites: [String: [Session]] is used in view whether each session is favorited or not, only.

if let isFavorited = session.isFavorited, isFavorited {
Image(systemName: "star.fill")
.foregroundColor(.yellow)
} else {
Image(systemName: "star")
.foregroundColor(.gray)
}
}

Expand Down
23 changes: 23 additions & 0 deletions MyLibrary/Sources/SharedModels/Conference.swift
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,12 @@ public struct Conference: Codable, Equatable, Hashable, Sendable {
self.date = date
self.schedules = schedules
}

public mutating func toggleFavorite(of session: Session) {
for index in schedules.indices {
schedules[index].toggleFavorite(of: session)
}
}
}

public struct Schedule: Codable, Equatable, Hashable, Sendable {
Expand All @@ -20,6 +26,12 @@ public struct Schedule: Codable, Equatable, Hashable, Sendable {
self.time = time
self.sessions = sessions
}

mutating func toggleFavorite(of session: Session) {
for index in sessions.indices {
sessions[index].toggleFavoriteIfEqual(with: session)
}
}
}

public struct Session: Codable, Equatable, Hashable, Sendable {
Expand All @@ -29,6 +41,7 @@ public struct Session: Codable, Equatable, Hashable, Sendable {
public var place: String?
public var description: String?
public var requirements: String?
public var isFavorited: Bool?

public init(
title: String, speakers: [Speaker]?, place: String?, description: String?,
Expand All @@ -40,4 +53,14 @@ public struct Session: Codable, Equatable, Hashable, Sendable {
self.description = description
self.requirements = requirements
}

mutating func toggleFavoriteIfEqual(with session: Session) {
if self == session {
if let isFavorited = isFavorited, isFavorited {
self.isFavorited = false
} else {
self.isFavorited = true
}
}
}
}