Feature/445 stretchfield#603
Conversation
… feature/445-stretchfield
… feature/445-stretchfield
|
This still needs more work. For some reason other fields are getting double entries. |
|
Fixed build issue (from merge) and fixed colors for extended fields. Ready for some prelim review... what do folks think of this kind of implementation? |
|
After talking discussions about what the website is doing, I decided to try and extend just the lunch field. Still needs to be discussed with the mobile team to see if this change is worth keeping. |
|
Let's go ahead and review this one and merge if possible. |
MrSmart00
left a comment
MrSmart00
left a comment
There was a problem hiding this comment.
Thanks!!
I left some comments!
Please check them🙏
|
|
||
| public init( | ||
| timetableItem: TimetableItem, | ||
| cellCount: Int? = 1, |
There was a problem hiding this comment.
I think this argument is no needed an optional 🤔
and required property is no needed a default value 👍
| cellCount: Int? = 1, | |
| cellCount: Int, |
| Text("\(timetableItem.startsTimeString) - \(timetableItem.endsTimeString)") | ||
| .textStyle(.labelMedium) | ||
| .foregroundStyle(timetableItem.room.roomTheme.primaryColor) | ||
| .foregroundStyle(cellCount>1 ? AssetColors.Surface.onSurfaceVariant.swiftUIColor : timetableItem.room.roomTheme.primaryColor) |
There was a problem hiding this comment.
| .foregroundStyle(cellCount>1 ? AssetColors.Surface.onSurfaceVariant.swiftUIColor : timetableItem.room.roomTheme.primaryColor) | |
| .foregroundStyle(cellCount > 1 ? AssetColors.Surface.onSurfaceVariant.swiftUIColor : timetableItem.room.roomTheme.primaryColor) |
I want add spaces for any operators left and right😅
|
|
||
| extension RoomType { | ||
| func toRoom() -> TimetableRoom { | ||
| public func toRoom() -> TimetableRoom { |
There was a problem hiding this comment.
Why did you add this public🤔？
I seem this PR doesn't reference this method.
There was a problem hiding this comment.
I was using it elsewhere, but on closer inspection it was in the same file so public was not needed.
|
@MrSmart00 Thanks for your comments! I responded and made some adjustments. |
MrSmart00
left a comment
MrSmart00
left a comment
There was a problem hiding this comment.
left some comments~
please check them🙏
|
|
||
| extension TimetableTimeGroupItems { | ||
| func getCellForRoom(room: RoomType, onTap: @escaping (TimetableItemWithFavorite) -> Void) -> TimetableGridCard? { | ||
| func getCellForRoom(room: RoomType, cellCount: Int?=1, onTap: @escaping (TimetableItemWithFavorite) -> Void) -> TimetableGridCard? { |
There was a problem hiding this comment.
| func getCellForRoom(room: RoomType, cellCount: Int?=1, onTap: @escaping (TimetableItemWithFavorite) -> Void) -> TimetableGridCard? { | |
| func getCellForRoom(room: RoomType, cellCount: 1, onTap: @escaping (TimetableItemWithFavorite) -> Void) -> TimetableGridCard? { |
There was a problem hiding this comment.
I might be wrong, but I think the intended change is removing "?=1" not the Int part... correct?
There was a problem hiding this comment.
sorry it's wrong😅
func getCellForRoom(room: RoomType, cellCount: Int, onTap: @escaping (TimetableItemWithFavorite) -> Void) -> TimetableGridCard? {
is correct👍
@charles-b-stb
| ) { | ||
| self.timetableItem = timetableItem | ||
| self.onTap = onTap | ||
| self.cellCount = cellCount ?? 1 |
There was a problem hiding this comment.
you have ?? 1 is never used used warning yet.
you should remove it👍
Ok. I will commit this one as-is. Thanks! Co-authored-by: Hiroya Hinomori <nexearth@gmail.com>
2 participants
Issue
Overview (Required)
TODO: The cell spacing needs adjustment.TODO: The stretched cells should have the room color removed.colors fixedBased off #367 and should not be merged until after the grid merge is complete.
Links
Screenshot (Optional if screenshot test is present or unrelated to UI)
Movie (Optional)