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.

Sign up for GitHub

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

feat: Course Level Error Handling for Empty States #477

Closed
wants to merge 8 commits into from
Closed

feat: Course Level Error Handling for Empty States #477

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
e6f8e74
feat: Course Level Error Handling for Empty States
shafqat-muneer Jul 2, 2024
58dcf71
chore: address review feedback
shafqat-muneer Jul 5, 2024
4ddfc7d
chore: address review feedback
shafqat-muneer Jul 5, 2024
b8b3abc
chore: address review feedback
shafqat-muneer Jul 9, 2024
d061772
chore: resolved manual conflicts
shafqat-muneer Jul 9, 2024
aea50e7
chore: address review feedback
shafqat-muneer Jul 9, 2024
422c1ed
chore: address review feedback
shafqat-muneer Jul 9, 2024
3e8518a
chore: address review feedback
shafqat-muneer Jul 9, 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
15 changes: 15 additions & 0 deletions Core/Core/Assets.xcassets/CourseNavigationBar/noVideos.imageset/Contents.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
{
"images" : [
{
"filename" : "noVideos.svg",
"idiom" : "universal"
}
],
"info" : {
"author" : "xcode",
"version" : 1
},
"properties" : {
"preserves-vector-representation" : true
}
}
3 changes: 3 additions & 0 deletions Core/Core/Assets.xcassets/CourseNavigationBar/noVideos.imageset/noVideos.svg
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
15 changes: 15 additions & 0 deletions Core/Core/Assets.xcassets/Handouts/noAnnouncements.imageset/Contents.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
{
"images" : [
{
"filename" : "campaign_24dp_FILL0_wght400_GRAD0_opsz24.svg",
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please change the file name as per the asset name?

"idiom" : "universal"
}
],
"info" : {
"author" : "xcode",
"version" : 1
},
"properties" : {
"preserves-vector-representation" : true
}
}
1 change: 1 addition & 0 deletions .../Handouts/noAnnouncements.imageset/campaign_24dp_FILL0_wght400_GRAD0_opsz24.svg
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
15 changes: 15 additions & 0 deletions Core/Core/Assets.xcassets/Handouts/noHandouts.imageset/Contents.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
{
"images" : [
{
"filename" : "noHandouts.svg",
"idiom" : "universal"
}
],
"info" : {
"author" : "xcode",
"version" : 1
},
"properties" : {
"preserves-vector-representation" : true
}
}
3 changes: 3 additions & 0 deletions Core/Core/Assets.xcassets/Handouts/noHandouts.imageset/noHandouts.svg
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
15 changes: 15 additions & 0 deletions Core/Core/Assets.xcassets/information.imageset/Contents.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
{
"images" : [
{
"filename" : "Vector.svg",
"idiom" : "universal"
}
],
"info" : {
"author" : "xcode",
"version" : 1
},
"properties" : {
"preserves-vector-representation" : true
}
}
3 changes: 3 additions & 0 deletions Core/Core/Assets.xcassets/information.imageset/Vector.svg
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
4 changes: 4 additions & 0 deletions Core/Core/SwiftGen/Assets.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ public enum CoreAssets {
public static let downloads = ImageAsset(name: "downloads")
public static let home = ImageAsset(name: "home")
public static let more = ImageAsset(name: "more")
public static let noVideos = ImageAsset(name: "noVideos")
public static let videos = ImageAsset(name: "videos")
public static let dashboardEmptyPage = ImageAsset(name: "DashboardEmptyPage")
public static let addComment = ImageAsset(name: "addComment")
Expand Down Expand Up @@ -72,6 +73,8 @@ public enum CoreAssets {
public static let stopDownloading = ImageAsset(name: "stopDownloading")
public static let announcements = ImageAsset(name: "announcements")
public static let handouts = ImageAsset(name: "handouts")
public static let noAnnouncements = ImageAsset(name: "noAnnouncements")
public static let noHandouts = ImageAsset(name: "noHandouts")
public static let dashboard = ImageAsset(name: "dashboard")
public static let discovery = ImageAsset(name: "discovery")
public static let learn = ImageAsset(name: "learn")
Expand Down Expand Up @@ -109,6 +112,7 @@ public enum CoreAssets {
public static let favorite = ImageAsset(name: "favorite")
public static let finishedSequence = ImageAsset(name: "finished_sequence")
public static let goodWork = ImageAsset(name: "goodWork")
public static let information = ImageAsset(name: "information")
public static let learnEmpty = ImageAsset(name: "learn_empty")
public static let airmail = ImageAsset(name: "airmail")
public static let defaultMail = ImageAsset(name: "defaultMail")
Expand Down
6 changes: 5 additions & 1 deletion Core/Core/View/Base/DynamicOffsetView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,19 @@ public struct DynamicOffsetView: View {

@Binding private var coordinate: CGFloat
@Binding private var collapsed: Bool
@Binding private var viewHeight: CGFloat
@State private var collapseHeight: CGFloat = .zero

@Environment(\.isHorizontal) private var isHorizontal

public init(
coordinate: Binding<CGFloat>,
collapsed: Binding<Bool>
collapsed: Binding<Bool>,
viewHeight: Binding<CGFloat>
) {
self._coordinate = coordinate
self._collapsed = collapsed
self._viewHeight = viewHeight
}

public var body: some View {
Expand Down Expand Up @@ -73,5 +76,6 @@ public struct DynamicOffsetView: View {
? (isHorizontal ? collapsedHorizontalHeight : collapsedVerticalHeight)
: expandedHeight
)
viewHeight = collapseHeight
}
}
122 changes: 79 additions & 43 deletions Core/Core/View/Base/FullScreenErrorView/FullScreenErrorView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,15 @@ import Theme

public struct FullScreenErrorView: View {

public enum ErrorType {
public enum ErrorType: Equatable {
case noInternet
case noInternetWithReload
case generic
case noVideos(error: String, image: SwiftUI.Image)
case noHandouts(error: String, image: SwiftUI.Image)
case noAnnouncements(error: String, image: SwiftUI.Image)
case noCourseDates(error: String, image: SwiftUI.Image)
case noCourseware(error: String, image: SwiftUI.Image)
Copy link
Contributor

Choose a reason for hiding this comment

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

We are passing the same data in all the cases, how about defining a single case for all the cases and use that? Thoughts?

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 was thinking the same thing, but I opted for this approach to accommodate any future type-specific UI requirements. We can opt for single case for now. Thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

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

Opting for a single case is good at the moment. We will update it in the future as per the need.

}

private let errorType: ErrorType
Expand All @@ -34,56 +39,87 @@ public struct FullScreenErrorView: View {
}

public var body: some View {
GeometryReader { proxy in
VStack(spacing: 28) {
Spacer()
switch errorType {
case .noInternet, .noInternetWithReload:
CoreAssets.noWifi.swiftUIImage
switch errorType {
case .noVideos(error: let error, image: let image),
.noHandouts(error: let error, image: let image),
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove the extra indentation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

XCode indenting it like this way.

.noAnnouncements(error: let error, image: let image),
.noCourseDates(error: let error, image: let image),
.noCourseware(error: let error, image: let image):
GeometryReader { proxy in
VStack(spacing: 20) {
Spacer()
image
.resizable()
.renderingMode(.template)
.foregroundStyle(Color.primary)
.scaledToFit()
.foregroundStyle(Theme.Colors.textSecondary)
.aspectRatio(contentMode: .fit)
.frame(maxWidth: 72, maxHeight: 80)

Text(CoreLocalization.Error.Internet.noInternetTitle)
.font(Theme.Fonts.titleLarge)
.foregroundColor(Theme.Colors.textPrimary)

Text(CoreLocalization.Error.Internet.noInternetDescription)
.font(Theme.Fonts.bodyLarge)
.foregroundColor(Theme.Colors.textPrimary)
.multilineTextAlignment(.center)
.padding(.horizontal, 50)
case .generic:
CoreAssets.notAvaliable.swiftUIImage
.renderingMode(.template)
.foregroundStyle(Color.primary)
.scaledToFit()

Text(CoreLocalization.View.Snackbar.tryAgainBtn)
.font(Theme.Fonts.titleLarge)
.foregroundColor(Theme.Colors.textPrimary)

Text(CoreLocalization.Error.unknownError)
.font(Theme.Fonts.bodyLarge)
Text(error)
.font(Theme.Fonts.labelLarge)
.foregroundColor(Theme.Colors.textPrimary)
.multilineTextAlignment(.center)
.padding(.horizontal, 50)
Spacer()
}

if errorType != .noInternet {
UnitButtonView(
type: .reload,
action: {
self.action()
}
)
.frame(maxWidth: .infinity, maxHeight: proxy.size.height)
.background(
Theme.Colors.background
)
}
default:
GeometryReader { proxy in
VStack(spacing: 20) {
Spacer()
switch errorType {
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems bit odd to use the switch again under a case of the same switch. How about using a single GeometryReader and configure it according to the need.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for pointing that out. I have fixed it.

case .noInternet, .noInternetWithReload:
CoreAssets.noWifi.swiftUIImage
.renderingMode(.template)
.foregroundStyle(Theme.Colors.textSecondary)
.scaledToFit()

Text(CoreLocalization.Error.Internet.noInternetTitle)
.font(Theme.Fonts.labelLarge)
.foregroundColor(Theme.Colors.textPrimary)

Text(CoreLocalization.Error.Internet.noInternetDescription)
.font(Theme.Fonts.labelLarge)
.foregroundColor(Theme.Colors.textPrimary)
.multilineTextAlignment(.center)
.padding(.horizontal, 50)
case .generic:
CoreAssets.notAvaliable.swiftUIImage
.renderingMode(.template)
.foregroundStyle(Theme.Colors.textSecondary)
.scaledToFit()

Text(CoreLocalization.View.Snackbar.tryAgainBtn)
.font(Theme.Fonts.labelLarge)
.foregroundColor(Theme.Colors.textPrimary)

Text(CoreLocalization.Error.unknownError)
.font(Theme.Fonts.labelLarge)
.foregroundColor(Theme.Colors.textPrimary)
.multilineTextAlignment(.center)
.padding(.horizontal, 50)
default: EmptyView()
}

if errorType != .noInternet {
UnitButtonView(
type: .reload,
action: {
self.action()
}
)
}
Spacer()
}
Spacer()
.frame(maxWidth: .infinity, maxHeight: proxy.size.height)
.background(
Theme.Colors.background
)
}
.frame(maxWidth: .infinity, maxHeight: proxy.size.height)
.background(
Theme.Colors.background
)
}
}
}
Expand Down
7 changes: 7 additions & 0 deletions Course/Course/Presentation/Container/CourseContainerView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ public struct CourseContainerView: View {
@State private var coordinate: CGFloat = .zero
@State private var lastCoordinate: CGFloat = .zero
@State private var collapsed: Bool = false
@State private var viewHeight: CGFloat = .zero
@Environment(\.isHorizontal) private var isHorizontal
@Namespace private var animationNamespace
private var idiom: UIUserInterfaceIdiom { UIDevice.current.userInterfaceIdiom }
Expand Down Expand Up @@ -85,6 +86,7 @@ public struct CourseContainerView: View {
selection: $viewModel.selection,
coordinate: $coordinate,
collapsed: $collapsed,
viewHeight: $viewHeight,
dateTabIndex: CourseTab.dates.rawValue
)
} else {
Expand Down Expand Up @@ -182,6 +184,7 @@ public struct CourseContainerView: View {
selection: $viewModel.selection,
coordinate: $coordinate,
collapsed: $collapsed,
viewHeight: $viewHeight,
dateTabIndex: CourseTab.dates.rawValue
)
.tabItem {
Expand All @@ -199,6 +202,7 @@ public struct CourseContainerView: View {
selection: $viewModel.selection,
coordinate: $coordinate,
collapsed: $collapsed,
viewHeight: $viewHeight,
dateTabIndex: CourseTab.dates.rawValue
)
.tabItem {
Expand All @@ -212,6 +216,7 @@ public struct CourseContainerView: View {
courseID: courseID,
coordinate: $coordinate,
collapsed: $collapsed,
viewHeight: $viewHeight,
viewModel: courseDatesViewModel
)
.tabItem {
Expand All @@ -225,6 +230,7 @@ public struct CourseContainerView: View {
courseID: courseID,
coordinate: $coordinate,
collapsed: $collapsed,
viewHeight: $viewHeight,
viewModel: Container.shared.resolve(DiscussionTopicsViewModel.self,
argument: title)!,
router: Container.shared.resolve(DiscussionRouter.self)!
Expand All @@ -240,6 +246,7 @@ public struct CourseContainerView: View {
courseID: courseID,
coordinate: $coordinate,
collapsed: $collapsed,
viewHeight: $viewHeight,
viewModel: Container.shared.resolve(HandoutsViewModel.self, argument: courseID)!
)
.tabItem {
Expand Down
6 changes: 4 additions & 2 deletions Course/Course/Presentation/CourseRouter.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@ public protocol CourseRouter: BaseRouter {
handouts: String?,
announcements: [CourseUpdate]?,
router: Course.CourseRouter,
cssInjector: CSSInjector
cssInjector: CSSInjector,
type: HandoutsItemType
)

func showCourseComponent(
Expand Down Expand Up @@ -103,7 +104,8 @@ public class CourseRouterMock: BaseRouterMock, CourseRouter {
handouts: String?,
announcements: [CourseUpdate]?,
router: Course.CourseRouter,
cssInjector: CSSInjector
cssInjector: CSSInjector,
type: HandoutsItemType
) {}

public func showCourseComponent(
Expand Down
Loading
Loading