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

Add question/time limit quiz modes #3

Merged
merged 28 commits into from Nov 7, 2023
Merged

Add question/time limit quiz modes #3

merged 28 commits into from Nov 7, 2023

Conversation

twocentstudios
Copy link
Owner

@twocentstudios twocentstudios commented Nov 6, 2023

  • Add QuizModes: infinite, timeLimit, questionLimit.
  • Add DeterminateProgressView for showing time/question countdown.
  • Add PreSettingsView as a half-modal that appears within the TopicView context.
  • Add SessionSettingsClient to persist user quiz mode settings
  • Update TCA dependency from 1.0.0 to 1.3.0 for Sendable support.

Screenshots

Quiz Infinite Quiz Time Limit Quiz Question Limit
IMG_4513 IMG_4509 IMG_4512
Topic (Collapsed) Topic (Half Expanded) Session Summary
IMG_4507 IMG_4508 IMG_4510

Question Limit

count-question-limit.mov

Time Limit

count-time-limit.mov

@twocentstudios twocentstudios self-assigned this Nov 6, 2023
Comment on lines +10 to +13
enum QuizMode: Equatable, Identifiable, Codable, CaseIterable {
case infinite
case questionLimit
case timeLimit
Copy link
Owner Author

Choose a reason for hiding this comment

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

This QuizMode is separate from the top level QuizMode so that we can persist both questionLimit and timeLimit independently of the last chosen quizMode. There's a conversion function in the ListeningQuizFeature file.

Comment on lines +28 to +31
var showProgress: Bool
var showBiki: Bool
var showConfetti: Bool
var playHaptics: Bool
Copy link
Owner Author

Choose a reason for hiding this comment

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

I'll add full support for these in another PR.

Comment on lines +25 to +26
var questionLimit: Int
var timeLimit: Int // seconds
Copy link
Owner Author

Choose a reason for hiding this comment

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

I decided to save these values as raw Ints but only allow the user to choose from a few different values. In theory this could lead to validation issues down the road if the saved value is removed from the UI.

import DependenciesAdditions
import Foundation

struct SessionSettingsClient {
Copy link
Owner Author

Choose a reason for hiding this comment

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

Note: This file was technically renamed from the unused TopicSettingsClient for convenience.

@@ -13,45 +13,114 @@ struct BikiAnimation: Equatable {
let kind: Kind
}

extension ListeningQuizFeature.State {
Copy link
Owner Author

Choose a reason for hiding this comment

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

I moved some of the derived view values of the state into their own extension for organization purposes.

@@ -204,7 +222,7 @@ struct SessionSummaryView: View {
}
// TODO: Button: retry session with same settings (if timed session was completed)
Copy link
Owner Author

Choose a reason for hiding this comment

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

I considered adding a "Try Quiz Again" button, but I think it's better to add in a separate PR.

@PresentationState var quiz: ListeningQuizNavigationFeature.State?
@PresentationState var about: AboutFeature.State?
@PresentationState var destination: Destination.State?
Copy link
Owner Author

Choose a reason for hiding this comment

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

The PreSettingsView should always be visible as a detents sheet. However, SwiftUI breaks if you try to present another sheet/fullScreenCover while another sheet is presented. Therefore, in the reducer below, we have to jump through some hoops to ensure that the PreSettingsView is dismissed properly before AboutView or QuizView is presented, and then present it again once TopicView reappears.

state.about = .init()
state.destination = nil
return .run { send in
await send(.setDestination(.about(.init())))
Copy link
Owner Author

Choose a reason for hiding this comment

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

Doing this requires Destination.State and all its downstream types to be @Sendable. Way downstream the TCA built in AlertState type was not @Sendable in TCA v1.0.0, but it is in v1.3.0, so I updated TCA.

@@ -0,0 +1,39 @@
import SwiftUI

struct DeterminateProgressView: View {
Copy link
Owner Author

Choose a reason for hiding this comment

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

I implemented this view similarly to the earlier IndeterminateProgressView. Both could be implemented as ProgressViewStyle conforming types, but at this point I think they're more straightforward to read as simple Views.

@@ -433,22 +532,45 @@ struct ListeningQuizView: View {

@ViewBuilder func progressBar(viewStore: ViewStoreOf<ListeningQuizFeature>) -> some View {
HStack(spacing: 0) {
IndeterminateProgressView(
animationCount: viewStore.challengeCount,
color1: Color(.tintColor),
Copy link
Owner Author

Choose a reason for hiding this comment

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

This tintColor is way too prominent for the determinate variant, and I think it actually looks better as subtle gray anyway.

Comment on lines +216 to +226
.sheet(
store: store.scope(state: \.$destination, action: { .destination($0) }),
state: /TopicsFeature.Destination.State.preSettings,
action: TopicsFeature.Destination.Action.preSettings
) { store in
PreSettingsView(store: store)
.presentationDragIndicator(.visible)
.presentationDetents([.fraction(0.1), .medium, .large])
.presentationBackgroundInteraction(.enabled)
.interactiveDismissDisabled(true)
}
Copy link
Owner Author

Choose a reason for hiding this comment

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

I noticed a bug (documented on Mastondon) where SwiftUI presents this sheet as a fullScreenCover with no way to close it, but can't reproduce it.

At the time I saw the bug, the presentation modifiers were outside the ViewStore scope. I moved them inside (which I think is the correct placement) so hopefully that fixes it. There's not really much I can do to workaround it besides randomly placing delays, introspecting UIKit, or completely rewriting to not use sheet at all.

@twocentstudios twocentstudios marked this pull request as ready for review November 7, 2023 06:53
@twocentstudios twocentstudios merged commit abb5839 into main Nov 7, 2023
@twocentstudios twocentstudios deleted the time-attack branch November 7, 2023 06:56
@twocentstudios twocentstudios added this to the v1.2 milestone Nov 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant