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
2 changes: 1 addition & 1 deletion Core/Core/Assets.xcassets/Handouts/noAnnouncements.imageset/Contents.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"images" : [
{
"filename" : "campaign_24dp_FILL0_wght400_GRAD0_opsz24.svg",
"filename" : "noAnnouncements.svg",
"idiom" : "universal"
}
],
Expand Down
0 ...paign_24dp_FILL0_wght400_GRAD0_opsz24.svg → ...nnouncements.imageset/noAnnouncements.svg
View file
Open in desktop
File renamed without changes
139 changes: 62 additions & 77 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 @@ -39,88 +39,73 @@ public struct FullScreenErrorView: View {
}

public var body: some View {
switch errorType {
case .noVideos(error: let error, image: let image),
.noHandouts(error: let error, image: let image),
.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(Theme.Colors.textSecondary)
.aspectRatio(contentMode: .fit)
.frame(maxWidth: 72, maxHeight: 80)

Text(error)
.font(Theme.Fonts.labelLarge)
.foregroundColor(Theme.Colors.textPrimary)
.multilineTextAlignment(.center)
.padding(.horizontal, 50)
Spacer()
}
.frame(maxWidth: .infinity, maxHeight: proxy.size.height)
.background(
Theme.Colors.background
)
VStack(spacing: 20) {
Spacer()
switch errorType {
case .noVideos(error: let error, image: let image),
.noHandouts(error: let error, image: let image),
.noAnnouncements(error: let error, image: let image),
.noCourseDates(error: let error, image: let image),
.noCourseware(error: let error, image: let image):
image
.resizable()
.renderingMode(.template)
.foregroundStyle(Theme.Colors.textSecondary)
.aspectRatio(contentMode: .fit)
.frame(maxWidth: 72, maxHeight: 80)

Text(error)
.font(Theme.Fonts.labelLarge)
.foregroundColor(Theme.Colors.textPrimary)
.multilineTextAlignment(.center)
.padding(.horizontal, 50)
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:
GeometryReader { proxy in
VStack(spacing: 20) {
Spacer()
switch errorType {
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()
}
)
if errorType == .noInternetWithReload || errorType == .generic {
UnitButtonView(
type: .reload,
action: {
self.action()
}
Spacer()
}
.frame(maxWidth: .infinity, maxHeight: proxy.size.height)
.background(
Theme.Colors.background
)
}
Spacer()
}
.frame(maxWidth: .infinity, maxHeight: .infinity)
.background(
Theme.Colors.background
)
}
}

Expand Down
2 changes: 0 additions & 2 deletions Course/Course/Presentation/Container/CourseContainerViewModel.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,8 +186,6 @@ public class CourseContainerViewModel: BaseCourseViewModel {
if isInternetAvaliable {
courseStructure = try await interactor.getCourseBlocks(courseID: courseID)
NotificationCenter.default.post(name: .getCourseDates, object: courseID)
isShowProgress = false
isShowRefresh = false
if let courseStructure {
try await getResumeBlock(
courseID: courseID,
Expand Down
3 changes: 2 additions & 1 deletion Course/Course/Presentation/Handouts/HandoutsUpdatesDetailView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,8 @@ public struct HandoutsUpdatesDetailView: View {
.padding(.top, 8)
.frame(
maxHeight: .infinity,
alignment: .topLeading)
alignment: .topLeading
)
.onRightSwipeGesture {
router.back()
}
Expand Down
6 changes: 4 additions & 2 deletions Course/Course/Presentation/Handouts/HandoutsView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,8 @@ struct HandoutsView: View {
announcements: nil,
router: viewModel.router,
cssInjector: viewModel.cssInjector,
type: type)
type: type
)
viewModel.analytics.trackCourseEvent(
.courseHandouts,
biValue: .courseHandouts,
Expand All @@ -72,7 +73,8 @@ struct HandoutsView: View {
announcements: viewModel.updates,
router: viewModel.router,
cssInjector: viewModel.cssInjector,
type: type)
type: type
)
viewModel.analytics.trackCourseEvent(
.courseAnnouncement,
biValue: .courseAnnouncement,
Expand Down
71 changes: 41 additions & 30 deletions Discussion/Discussion/Presentation/DiscussionTopics/DiscussionTopicsView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,37 +54,39 @@ public struct DiscussionTopicsView: View {
bannerDiscussionsDisabled
}

HStack(spacing: 11) {
Image(systemName: "magnifyingglass")
.foregroundColor(Theme.Colors.textInputTextColor)
.padding(.leading, 16)
.padding(.top, 1)
Text(DiscussionLocalization.Topics.search)
.foregroundColor(Theme.Colors.textInputTextColor)
.font(Theme.Fonts.bodyMedium)
Spacer()
}
.frame(minHeight: 48)
.background(
Theme.Shapes.textInputShape
.fill(Theme.Colors.textInputBackground)
)
.overlay(
Theme.Shapes.textInputShape
.stroke(lineWidth: 1)
.fill(Theme.Colors.textInputUnfocusedStroke)
)
.onTapGesture {
viewModel.router.showDiscussionsSearch(
courseID: courseID,
isBlackedOut: viewModel.isBlackedOut
if let topics = viewModel.discussionTopics, topics.count > 0 {
HStack(spacing: 11) {
Image(systemName: "magnifyingglass")
.foregroundColor(Theme.Colors.textInputTextColor)
.padding(.leading, 16)
.padding(.top, 1)
Text(DiscussionLocalization.Topics.search)
.foregroundColor(Theme.Colors.textInputTextColor)
.font(Theme.Fonts.bodyMedium)
Spacer()
}
.frame(minHeight: 48)
.background(
Theme.Shapes.textInputShape
.fill(Theme.Colors.textInputBackground)
)
.overlay(
Theme.Shapes.textInputShape
.stroke(lineWidth: 1)
.fill(Theme.Colors.textInputUnfocusedStroke)
)
.onTapGesture {
viewModel.router.showDiscussionsSearch(
courseID: courseID,
isBlackedOut: viewModel.isBlackedOut
)
}
.frameLimit(width: proxy.size.width)
.padding(.horizontal, 24)
.padding(.top, 10)
.accessibilityElement(children: .ignore)
.accessibilityLabel(DiscussionLocalization.Topics.search)
}
.frameLimit(width: proxy.size.width)
.padding(.horizontal, 24)
.padding(.top, 10)
.accessibilityElement(children: .ignore)
.accessibilityLabel(DiscussionLocalization.Topics.search)

// MARK: - Page Body
VStack {
Expand Down Expand Up @@ -160,7 +162,16 @@ public struct DiscussionTopicsView: View {
}
}
}

} else if viewModel.isShowProgress == false {
FullScreenErrorView(
type: .noCourseDates(
error: DiscussionLocalization.Error.unableToLoadCourseContent,
image: CoreAssets.information.swiftUIImage
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of passing message and icon from everywhere, how about just setting the type and define a method in FullScreenErrorView for message and icon. You can keep a generic type that will take message and icon as in input for future.

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 FullScreenErrorView is utilized across various modules, with its implementation located in the Core module to enable application-wide usage. Embedding messages within the full screen would necessitate moving module-specific errors into the core module, which is not ideal. Therefore, I suggest passing the message and icon instead.

)
)
.frame(maxWidth: .infinity)
.frame(height: proxy.size.height - viewHeight)
Spacer(minLength: -200)
}
Spacer(minLength: 200)
}
Expand Down
5 changes: 5 additions & 0 deletions Discussion/Discussion/SwiftGen/Strings.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,11 @@ public enum DiscussionLocalization {
/// Topic
public static let topic = DiscussionLocalization.tr("Localizable", "CREATE_THREAD.TOPIC", fallback: "Topic")
}
public enum Error {
/// Unable to load course content.
/// Try again later.
public static let unableToLoadCourseContent = DiscussionLocalization.tr("Localizable", "ERROR.UNABLE_TO_LOAD_COURSE_CONTENT", fallback: "Unable to load course content.\nTry again later.")
}
public enum Post {
/// Last post:
public static let lastPost = DiscussionLocalization.tr("Localizable", "POST.LAST_POST", fallback: "Last post:")
Expand Down
4 changes: 3 additions & 1 deletion Discussion/Discussion/en.lproj/Localizable.strings
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
"COMMENT.REPORT" = "Report";
"COMMENT.UNREPORT" = "Unreport";
"COMMENT.FOLLOW" = "Follow";
"COMMENT.UNFOLLOW" = "Unfollow";
"COMMENT.UNFOLLOW" = "Unfollow";//

"RESPONSE.COMMENTS_RESPONSES" = "Comment";
"RESPONSE.ALERT.COMMENT_ADDED" = "Comment added";
Expand All @@ -61,3 +61,5 @@
"SEARCH.EMPTY_DESCRIPTION" = "Start typing to find the topics";

"ANONYMOUS" = "Anonymous";

"ERROR.UNABLE_TO_LOAD_COURSE_CONTENT" = "Unable to load course content.\nTry again later.";
2 changes: 2 additions & 0 deletions Discussion/Discussion/uk.lproj/Localizable.strings
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,3 +57,5 @@
"POST_TYPE.DISCUSSION" = "дискусія";

"ANONYMOUS" = "Анонім";

"ERROR.UNABLE_TO_LOAD_COURSE_CONTENT" = "Unable to load course content.\nTry again later.";
Loading