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

Split PirateCoinsPage into 2 Pages #244

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 6 additions & 3 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
# Env managment
lib/dart_define.gen.dart
# start of code generated by package:dart_define, DO NOT modify or remove these comments
dart_define.json
# end of code generated by package:dart_define, DO NOT modify or remove these comments

Copy link
Member

Choose a reason for hiding this comment

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

Why were these changed?

Copy link
Contributor

Choose a reason for hiding this comment

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

nothing I did purposely

Copy link
Member

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.


# l10n
lib/l10n/app_localizations*.dart
Expand Down Expand Up @@ -175,3 +173,8 @@ fabric.properties
# Android studio 3.1+ serialized cache file
.idea/caches/build_file_checksums.ser


# start of code generated by package:dart_define, DO NOT modify or remove these comments
dart_define.json
dart_define.json
# end of code generated by package:dart_define, DO NOT modify or remove these comments
19 changes: 16 additions & 3 deletions lib/app/app_router.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

The 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.

Copy link
Contributor

Choose a reason for hiding this comment

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

alr

children: [
AutoRoute(
page: StatsRoute.page,
Expand Down
3 changes: 2 additions & 1 deletion lib/features/dashboard/application/dashboard_service.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

The 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,
Expand Down
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
Expand Up @@ -19,15 +19,16 @@ import "../../domain/coins_model.dart";

/// The page located at `/pirate-coins`.
@RoutePage()
class PirateCoinsPage extends ConsumerWidget {
/// Create a new instance of [PirateCoinsPage].
const PirateCoinsPage({super.key});
class PirateCoinsTeacherPage extends ConsumerWidget {
/// Create a new instance of [PirateCoinsTeacherPage].
const PirateCoinsTeacherPage({super.key});

@override
Widget build(BuildContext context, WidgetRef ref) {
final accountType = ref.watch(accountTypeProvider).valueOrNull;

return Center(
/// If the user is not a teacher, redirect them to the student page.
child: switch (accountType) {
AccountType.student => const _StudentView(),
AccountType.teacher => const _TeacherView(),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
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_page.dart";
import "package:pirate_code/features/pirate_coins/presentation/pirate_coins_page/pirate_coins_teacher_page.dart";

import "../../../../helpers/pump_app.dart";

Expand All @@ -16,7 +16,7 @@ void main() {
overrides: [
authProvider.overrideWithValue(mockAuthRepository),
],
const PirateCoinsPage(),
const PirateCoinsTeacherPage(),
);

verify(() => mockAuthRepository.authenticate(anonymous: true))
Expand All @@ -34,7 +34,7 @@ void main() {
overrides: [
authProvider.overrideWithValue(mockAuthRepository),
],
const PirateCoinsPage(),
const PirateCoinsTeacherPage(),
);

verify(() => mockAuthRepository.authenticate(anonymous: true))
Expand All @@ -52,7 +52,7 @@ void main() {
overrides: [
authProvider.overrideWithValue(mockAuthRepository),
],
const PirateCoinsPage(),
const PirateCoinsTeacherPage(),
);

verify(() => mockAuthRepository.authenticate(anonymous: true))
Expand All @@ -70,7 +70,7 @@ void main() {
overrides: [
authProvider.overrideWithValue(mockAuthRepository),
],
const PirateCoinsPage(),
const PirateCoinsTeacherPage(),
);

verify(() => mockAuthRepository.authenticate(anonymous: true))
Expand Down
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 {}
Loading