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

Feature/timetable feature grid2 #367

Merged
merged 29 commits into from
Aug 18, 2024
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
6be1dfa
WIP
charles-b-stb Aug 12, 2024
ab2c5e5
Merge branch 'main' of github.com:DroidKaigi/conference-app-2024 into…
charles-b-stb Aug 12, 2024
3d84fe1
wip
charles-b-stb Aug 12, 2024
42a0f8f
Grid showing, data not displayable yet.
charles-b-stb Aug 12, 2024
a8ec52f
Very basic grid in operation. Still needs to fix click operation. Sti…
charles-b-stb Aug 12, 2024
398d589
Code cleanup
charles-b-stb Aug 12, 2024
16190a5
Update app-ios/Sources/CommonComponents/Timetable/TimetableGridCard.s…
charles-b-stb Aug 13, 2024
3ec6e7f
Fixes as per pull request comments.
charles-b-stb Aug 13, 2024
7ce2c62
Merge branch 'main' of github.com:DroidKaigi/conference-app-2024 into…
charles-b-stb Aug 13, 2024
3458ce9
Merge branch 'main' of github.com:DroidKaigi/conference-app-2024 into…
charles-b-stb Aug 13, 2024
0c5fac1
Fixed build with suggestions.
charles-b-stb Aug 13, 2024
404fec3
Merge branch 'main' of github.com:DroidKaigi/conference-app-2024 into…
charles-b-stb Aug 14, 2024
7ef8e12
Update app-ios/Sources/TimetableFeature/TimetableListView.swift
charles-b-stb Aug 14, 2024
01a633e
Update app-ios/Sources/CommonComponents/Timetable/TimetableGridCard.s…
charles-b-stb Aug 14, 2024
c75137c
Update app-ios/Sources/CommonComponents/Timetable/TimetableGridCard.s…
charles-b-stb Aug 14, 2024
1085d24
Update app-ios/Sources/CommonComponents/Timetable/TimetableGridCard.s…
charles-b-stb Aug 14, 2024
38eaa49
Fixed various issues.
charles-b-stb Aug 14, 2024
34c3e46
Merge branch 'feature/timetable-feature-grid2' of github.com:DroidKai…
charles-b-stb Aug 14, 2024
f201358
Fixed various issues that came up during pull request.
charles-b-stb Aug 14, 2024
2a8d3eb
Merge branch 'main' of github.com:DroidKaigi/conference-app-2024 into…
charles-b-stb Aug 14, 2024
0900ab1
Merge branch 'main' into feature/timetable-feature-grid2
charles-b-stb Aug 14, 2024
f48e5ba
Replaced Spacer with Color as per PR comment
charles-b-stb Aug 16, 2024
2563d07
Merge branch 'feature/timetable-feature-grid2' of github.com:DroidKai…
charles-b-stb Aug 16, 2024
ab85531
Update app-ios/Sources/TimetableFeature/TimetableListView.swift
charles-b-stb Aug 17, 2024
bdf3b12
Update app-ios/Sources/CommonComponents/Timetable/TimetableGridCard.s…
charles-b-stb Aug 17, 2024
348203e
Update app-ios/Sources/TimetableFeature/TimetableDataItems.swift
charles-b-stb Aug 17, 2024
b39f2d6
Fixed items broken by pullrequest suggest commits.
charles-b-stb Aug 17, 2024
16262aa
Moved GridItem back into the Timetable module.
charles-b-stb Aug 18, 2024
45d96ec
Rewrote to move clear outside TimetableGridCard
charles-b-stb Aug 18, 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
88 changes: 88 additions & 0 deletions app-ios/Sources/CommonComponents/Timetable/TimetableGridCard.swift
Copy link
Contributor

Choose a reason for hiding this comment

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

This view isn't common component I think.
you should move to Timetable package please🙏

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I moved it, but it did require me to make one shared class between the card classes public (it was package before)

Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
import Foundation
import SwiftUI
import Theme
import class shared.TimetableItem

public struct TimetableGridCard: View {
let timetableItem: TimetableItem?
let onTap: (TimetableItem) -> Void

public init(
timetableItem: TimetableItem?,
onTap: @escaping (TimetableItem) -> Void
) {
self.timetableItem = timetableItem
self.onTap = onTap
}

public var body: some View {
if let timetableItem {
Button {
onTap(timetableItem)
} label: {
VStack(alignment: .leading, spacing: 8) {
HStack(spacing: 4) {
timetableItem.room.type.shape
.foregroundColor(timetableItem.room.roomTheme.primaryColor)
charles-b-stb marked this conversation as resolved.
Show resolved Hide resolved
Text("\(timetableItem.startsTimeString) - \(timetableItem.endsTimeString)")
.textStyle(.labelMedium)
.foregroundColor(timetableItem.room.roomTheme.primaryColor)
charles-b-stb marked this conversation as resolved.
Show resolved Hide resolved
Spacer()
}

Text(timetableItem.title.currentLangTitle)
.textStyle(.titleMedium)
.foregroundColor(timetableItem.room.roomTheme.primaryColor)
charles-b-stb marked this conversation as resolved.
Show resolved Hide resolved
.multilineTextAlignment(.leading)

Spacer()

ForEach(timetableItem.speakers, id: \.id) { speaker in
HStack(spacing: 8) {
Group {
AsyncImage(url: URL(string: speaker.iconUrl)) {
$0.resizable()
} placeholder: {
Color.gray
}
}
.frame(width: 32, height: 32)
.clipShape(Circle())

Text(speaker.name)
.textStyle(.titleSmall)
.foregroundStyle(AssetColors.Surface.onSurfaceVariant.swiftUIColor)
.lineLimit(1)
}
}
}
.frame(maxWidth: .infinity)
.padding(12)
.frame(width: 192, height: 153)
.background(timetableItem.room.roomTheme.containerColor, in: RoundedRectangle(cornerRadius: 4))
.overlay(RoundedRectangle(cornerRadius: 4).stroke(timetableItem.room.roomTheme.primaryColor, lineWidth: 1))
}
} else {
VStack {
//Empty on purpose
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to instead it to EmptyView() ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will try it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Empty view collapses. We need something that will hold the space open.

Copy link
Member

Choose a reason for hiding this comment

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

How about following?

        Color.clear
            .frame(maxWidth: .infinity)
            .padding(12)
            .frame(width: 192, height: 153)
            .clipShape(RoundedRectangle(cornerRadius: 4))

.frame(maxWidth: .infinity)
.padding(12)
.frame(width: 192, height: 153)
.background(Color.clear, in: RoundedRectangle(cornerRadius: 4))
charles-b-stb marked this conversation as resolved.
Show resolved Hide resolved
}

}
}

#Preview {
VStack {
TimetableGridCard(
timetableItem: TimetableItem.Session.companion.fake(),
onTap: { _ in }
)
.padding(.horizontal, 16)
}
.frame(maxWidth: .infinity, maxHeight: .infinity)
.background(Color.black)
}
7 changes: 7 additions & 0 deletions app-ios/Sources/TimetableFeature/SampleData.swift
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it correct my understanding that sample data no longer used?
If so, I don't think this file name is appropriate.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The sample data is still used in previews. I can refactor this to break out data class items.

Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import Foundation
import CommonComponents
import shared

public enum DayTab: String, CaseIterable, Identifiable, Sendable {
Expand Down Expand Up @@ -28,6 +29,12 @@ public struct TimetableTimeGroupItems: Identifiable, Equatable, Hashable {
self.endsTimeString = endsTimeString
self.items = items
}

func getItemForRoom(forRoom: RoomType) -> TimetableItemWithFavorite? {
items.filter {
$0.timetableItem.room.type == forRoom
}.first
}
}

// This exists only for previews now.
Expand Down
148 changes: 121 additions & 27 deletions app-ios/Sources/TimetableFeature/TimetableListView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,7 @@
case TimetableMode.list:
TimetableListView(store: store)
case TimetableMode.grid:
Text("Grid view placeholder")
.foregroundStyle(AssetColors.Surface.onSurface.swiftUIColor)
TimetableGridView(store: store)
}
Spacer()
}
Expand Down Expand Up @@ -107,6 +106,53 @@
}
}

struct TimetableGridView: View {
private let store: StoreOf<TimetableReducer>

public init(store: StoreOf<TimetableReducer>) {
self.store = store
}

var body: some View {
let rooms = RoomType.allCases.filter {$0 != RoomType.roomIj}

ScrollView([.horizontal, .vertical]) {
Grid {
GridRow {
Color.clear
.gridCellUnsizedAxes([.horizontal, .vertical])

ForEach(rooms, id: \.self) { column in

let room = getTimetableRoom(type: column)

Text(room.name.currentLangTitle).foregroundStyle(room.roomTheme.primaryColor)
.frame(width: 192)
}
}
ForEach(store.timetableItems, id: \.self) { timeBlock in
GridRow {
VStack {
Text(timeBlock.startsTimeString).foregroundStyle(AssetColors.Surface.onSurface.swiftUIColor)
Spacer()

}.frame(height: 153)

Check failure on line 140 in app-ios/Sources/TimetableFeature/TimetableListView.swift

View workflow job for this annotation

GitHub Actions / build-ios

member 'timetableItemTapped' expects argument of type 'TimetableItemWithFavorite'
charles-b-stb marked this conversation as resolved.
Show resolved Hide resolved

ForEach(rooms, id: \.self) { room in

timeBlock.getCellForRoom(room: room, onTap: { item in
store.send(.view(.timetableItemTapped))
Copy link
Member

Choose a reason for hiding this comment

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

TimetableItemWithFavorite instance should be passed to timetableItemTapped 🙏🏼

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I need to backmerge for this to work, so I will fix that soon.

})
}
}
}
}
}

}
}

struct TimeGroupMiniList: View {
let contents: TimetableTimeGroupItems
let onItemTap: (TimetableItemWithFavorite) -> Void
Expand Down Expand Up @@ -136,38 +182,86 @@
}
}

struct TagView: View {
let tagText: String
let highlight: Bool

var body: some View {
HStack {
if highlight {
Image(systemName: "diamond.fill").resizable().frame(width: 11,height: 11).foregroundStyle(AssetColors.Custom.flamingo.swiftUIColor)
.padding(-3)
}
Text(tagText).foregroundStyle(highlight ? AssetColors.Custom.flamingo.swiftUIColor : AssetColors.Surface.onSurface.swiftUIColor)
}
.padding(
EdgeInsets(top: 2,leading: 7, bottom: 2, trailing: 7))
.border(highlight ? AssetColors.Custom.flamingo.swiftUIColor : AssetColors.Surface.onSurface.swiftUIColor)
.padding(-2)
func getTimetableRoom(type: RoomType) -> TimetableRoom {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would be more Swift-Like to implement this convert logic in extension.
Also, you can narrow the scope in which this function can be used.

extension RoomType {
    func toRoom() -> TimetableRoom {
        ...
    }
}

let type: RoomType = .roomI
let room = type.toRoom()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

switch (type) {
charles-b-stb marked this conversation as resolved.
Show resolved Hide resolved
case .roomI:
return TimetableRoom(
id: 1,
name: MultiLangText(
jaTitle: "Iguana",
enTitle: "Iguana"
),
type: .roomI,
sort: 1
)
case .roomG:
return TimetableRoom(
id: 2,
name: MultiLangText(
jaTitle: "Giraffe",
enTitle: "Giraffe"
),
type: .roomG,
sort: 2
)
case .roomH:
return TimetableRoom(
id: 3,
name: MultiLangText(
jaTitle: "Hedgehog",
enTitle: "Hedgehog"
),
type: .roomH,
sort: 3
)
case .roomF:
return TimetableRoom(
id: 4,
name: MultiLangText(
jaTitle: "Flamingo",
enTitle: "Flamingo"
),
type: .roomF,
sort: 4
)
case .roomJ:
return TimetableRoom(
id: 5,
name: MultiLangText(
jaTitle: "Jellyfish",
enTitle: "Jellyfish"
),
type: .roomJ,
sort: 5
)
case .roomIj:
return TimetableRoom(
id: 6,
name: MultiLangText(
jaTitle: "All",
enTitle: "All"
Copy link
Contributor

Choose a reason for hiding this comment

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

According to KMP's implementation, It seems like room IJ's name is "Iguana and Jellyfish"👀
スクリーンショット 2024-08-14 1 10 49

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there not an item for all rooms? From the UI it looked like some events fill the whole screen.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Bigger question: Do we have any examples of Iguana and Jellyfish? It seems like a case we should test in grid mode.

),
type: .roomIj,
sort: 6
)
}
}

struct PhotoView: View {
//TODO: Replace this with an actual photo render
let photo: String
let name: String

var body: some View {
HStack {
Image(systemName:photo).resizable().frame(width: 32,height: 32).foregroundStyle(AssetColors.Custom.flamingo.swiftUIColor)
Text(name)
extension TimetableTimeGroupItems {
func getCellForRoom(room: RoomType, onTap: @escaping (TimetableItem) -> Void) -> TimetableGridCard {
return if let cell = getItemForRoom(forRoom: room) {
TimetableGridCard(timetableItem: cell.timetableItem) { timetableItem in
onTap(timetableItem)
}
} else {
TimetableGridCard(timetableItem: nil) { _ in
// Does nothing
}
}
}
}


#Preview {
TimetableView(
store: .init(initialState: .init(timetableItems: SampleData.init().workdayResults),
Expand Down
Loading