-
Notifications
You must be signed in to change notification settings - Fork 0
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
Split PirateCoinsPage
into 2 Pages
#244
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,7 +9,8 @@ import "../features/auth/presentation/auth_page/auth_page.dart"; | |
import "../features/dashboard/presentation/dashboard_page/dashboard_page.dart"; | ||
import "../features/dashboard/presentation/wrapper_page/wrapper_page.dart"; | ||
import "../features/gpa_calculator/presentation/gpa_page/gpa_page.dart"; | ||
import "../features/pirate_coins/presentation/pirate_coins_page/pirate_coins_page.dart"; | ||
import "../features/pirate_coins/presentation/pirate_coins_page/pirate_coins_student_page.dart"; | ||
import "../features/pirate_coins/presentation/pirate_coins_page/pirate_coins_teacher_page.dart"; | ||
import "../features/pirate_coins/presentation/stats_page/stats_page.dart"; | ||
|
||
part "app_router.gr.dart"; | ||
|
@@ -42,8 +43,20 @@ class AppRouter extends _$AppRouter { | |
], | ||
children: [ | ||
AutoRoute( | ||
page: PirateCoinsRoute.page, | ||
path: "pirate-coins", | ||
page: PirateCoinsTeacherRoute.page, | ||
path: "pirate-coins-teacher", | ||
children: [ | ||
AutoRoute( | ||
page: StatsRoute.page, | ||
path: "stats", | ||
title: (context, route) => "Stats", | ||
), | ||
], | ||
title: (context, route) => "Pirate Coins", | ||
), | ||
AutoRoute( | ||
page: PirateCoinsStudentRoute.page, | ||
path: "pirate-coins-student", | ||
Comment on lines
+46
to
+59
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would be better for these to be nested and the difference in routes invisible, at least to students. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. alr |
||
children: [ | ||
AutoRoute( | ||
page: StatsRoute.page, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -35,7 +35,8 @@ List<AppletEntity> get _applets { | |
image: appletsFolder.pirateCoins.path, | ||
color: const Color.fromARGB(255, 122, 194, 129), | ||
// TODO(lishaduck): figure out how to use routes to keep this type-safe. | ||
location: "/pirate-coins", | ||
location: | ||
"/pirate-coins-student", //not sure how to make this dynamic based on account type | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using route guards should remove to need to hardcode to student. And yes, I know it's not possible right now, which is why I'm working on #253. |
||
), | ||
AppletEntity( | ||
image: appletsFolder.gpaCalculator.path, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,79 @@ | ||
/// This library contains the Pirate Coins feature's main page. | ||
library; | ||
|
||
import "package:auto_route/auto_route.dart"; | ||
import "package:flutter/material.dart"; | ||
import "package:hooks_riverpod/hooks_riverpod.dart"; | ||
|
||
import "../../../../l10n/l10n.dart"; | ||
import "../../../../widgets/big_card/big_card.dart"; | ||
// import "../../../auth/domain/account_type.dart"; | ||
import "../../application/coins_service.dart"; | ||
import "../../domain/coins_model.dart"; | ||
|
||
/// The page located at `/pirate-coins`. | ||
@RoutePage() | ||
class PirateCoinsStudentPage extends ConsumerWidget { | ||
/// Create a new instance of [PirateCoinsStudentPage]. | ||
const PirateCoinsStudentPage({super.key}); | ||
|
||
@override | ||
Widget build(BuildContext context, WidgetRef ref) { | ||
// final accountType = ref.watch(accountTypeProvider).valueOrNull; | ||
|
||
return const Center( | ||
/// If the user is not a teacher, redirect them to the student page. | ||
child: _StudentView(), | ||
); | ||
} | ||
} | ||
|
||
class _StudentView extends ConsumerWidget { | ||
const _StudentView({ | ||
// Temporary ignore, see <dart-lang/sdk#49025>. | ||
// ignore: unused_element | ||
super.key, | ||
}); | ||
|
||
@override | ||
Widget build(BuildContext context, WidgetRef ref) { | ||
final data = ref.watch(currentUserCoinsProvider); | ||
|
||
return Column( | ||
mainAxisAlignment: MainAxisAlignment.center, | ||
children: [ | ||
_ViewCoins(data: data), | ||
], | ||
); | ||
} | ||
} | ||
|
||
class _ViewCoins extends StatelessWidget { | ||
const _ViewCoins({ | ||
required this.data, | ||
// Temporary ignore, see <dart-lang/sdk#49025>. | ||
// ignore: unused_element | ||
super.key, | ||
}); | ||
|
||
final AsyncValue<CoinsModel> data; | ||
|
||
@override | ||
Widget build(BuildContext context) { | ||
final l10n = context.l10n; | ||
|
||
return Padding( | ||
padding: const EdgeInsets.all(8), | ||
child: switch (data) { | ||
AsyncData(:final value) => BigCard("${value.coins.coins}"), | ||
AsyncError(:final error) => BigCard(l10n.error("$error")), | ||
AsyncLoading() => Column( | ||
children: [ | ||
const CircularProgressIndicator(), | ||
Text(l10n.loading), | ||
], | ||
), | ||
}, | ||
); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,87 @@ | ||
import "package:flutter_test/flutter_test.dart"; | ||
import "package:mocktail/mocktail.dart"; | ||
import "package:pirate_code/features/auth/data/auth_repository.dart"; | ||
import "package:pirate_code/features/pirate_coins/presentation/pirate_coins_page/pirate_coins_student_page.dart"; | ||
|
||
import "../../../../helpers/pump_app.dart"; | ||
|
||
void main() { | ||
group("Pirate Coins page is accessible...", () { | ||
group("is accessible...", () { | ||
testWidgets("on Android.", (tester) async { | ||
final mockAuthRepository = _MockAuthRepository(); | ||
verifyZeroInteractions(mockAuthRepository); | ||
|
||
await tester.pumpApp( | ||
overrides: [ | ||
authProvider.overrideWithValue(mockAuthRepository), | ||
], | ||
const PirateCoinsStudentPage(), | ||
); | ||
|
||
verify(() => mockAuthRepository.authenticate(anonymous: true)) | ||
.called(1); | ||
|
||
final handle = tester.ensureSemantics(); | ||
await expectLater(tester, meetsGuideline(androidTapTargetGuideline)); | ||
handle.dispose(); | ||
}); | ||
testWidgets("on iOS.", (tester) async { | ||
final mockAuthRepository = _MockAuthRepository(); | ||
verifyZeroInteractions(mockAuthRepository); | ||
|
||
await tester.pumpApp( | ||
overrides: [ | ||
authProvider.overrideWithValue(mockAuthRepository), | ||
], | ||
const PirateCoinsStudentPage(), | ||
); | ||
|
||
verify(() => mockAuthRepository.authenticate(anonymous: true)) | ||
.called(1); | ||
|
||
final handle = tester.ensureSemantics(); | ||
await expectLater(tester, meetsGuideline(iOSTapTargetGuideline)); | ||
handle.dispose(); | ||
}); | ||
testWidgets("according to the WCAG.", (tester) async { | ||
final mockAuthRepository = _MockAuthRepository(); | ||
verifyZeroInteractions(mockAuthRepository); | ||
|
||
await tester.pumpApp( | ||
overrides: [ | ||
authProvider.overrideWithValue(mockAuthRepository), | ||
], | ||
const PirateCoinsStudentPage(), | ||
); | ||
|
||
verify(() => mockAuthRepository.authenticate(anonymous: true)) | ||
.called(1); | ||
|
||
final handle = tester.ensureSemantics(); | ||
await expectLater(tester, meetsGuideline(textContrastGuideline)); | ||
handle.dispose(); | ||
}); | ||
testWidgets("with regard to labeling buttons.", (tester) async { | ||
final mockAuthRepository = _MockAuthRepository(); | ||
verifyZeroInteractions(mockAuthRepository); | ||
|
||
await tester.pumpApp( | ||
overrides: [ | ||
authProvider.overrideWithValue(mockAuthRepository), | ||
], | ||
const PirateCoinsStudentPage(), | ||
); | ||
|
||
verify(() => mockAuthRepository.authenticate(anonymous: true)) | ||
.called(1); | ||
|
||
final handle = tester.ensureSemantics(); | ||
await expectLater(tester, meetsGuideline(labeledTapTargetGuideline)); | ||
handle.dispose(); | ||
}); | ||
}); | ||
}); | ||
} | ||
|
||
class _MockAuthRepository extends Mock implements AuthRepository {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why were these changed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nothing I did purposely
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm removing it in #256 anyway, so it's not a big deal.