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

Fix up the Teacher view #192

Open
Tracked by #10
PetalCat opened this issue Sep 13, 2023 · 7 comments · May be fixed by #244
Open
Tracked by #10

Fix up the Teacher view #192

PetalCat opened this issue Sep 13, 2023 · 7 comments · May be fixed by #244
Assignees
Labels
dart Regards Dart code enhancement New feature or request
Milestone

Comments

@PetalCat
Copy link
Contributor

PetalCat commented Sep 13, 2023

We need to separate the student and teacher pages.

@PetalCat PetalCat mentioned this issue Sep 13, 2023
@PetalCat PetalCat added the enhancement New feature or request label Sep 13, 2023
@lishaduck
Copy link
Member

lishaduck commented Sep 13, 2023

Yep! The current system is split up into lots of small, modular widgets so that they can be on separate pages.
Ideally, this change would also make the teacher page store its state in the URL, rather than in state management.

@lishaduck lishaduck changed the title Add a Teacher view Fix up the Teacher view Sep 13, 2023
@lishaduck lishaduck added the dart Regards Dart code label Sep 13, 2023
@lishaduck lishaduck added this to the Summer Work milestone Sep 13, 2023
@lishaduck
Copy link
Member

You wanna tackle this next after #193, @ParkerH27?

@lishaduck
Copy link
Member

lishaduck commented Oct 1, 2023

Made a few changes to the surrounding code to make this a tad bit easier.

@PetalCat
Copy link
Contributor Author

PetalCat commented Oct 4, 2023

You wanna tackle this next after #193, @ParkerH27?

I guess so.... #193 is brain breaking

@lishaduck
Copy link
Member

It's been 2 months. You still up for this? We need to get it done before we present (b/c it's blocking me on some other stuff).

PetalCat added a commit that referenced this issue Nov 20, 2023
@PetalCat PetalCat linked a pull request Nov 20, 2023 that will close this issue
@ParkerH27
Copy link
Contributor

A: Hopefully that is what I was supposed to do because it felt waaayyy too easy(that is scary)
B: You're welcome for somehow failing the Markdown test :)

lishaduck pushed a commit that referenced this issue Nov 22, 2023
@lishaduck
Copy link
Member

lishaduck commented Nov 22, 2023

A: Hopefully that is what I was supposed to do because it felt waaayyy too easy(that is scary)
B: You're welcome for somehow failing the Markdown test :)

I'll address both of these in turn, @PetalCat.

  • B: It's a bug, something changed on GitHub's side. I've removed the auto-pr to fix markdownlint failures, so it works now.
  • A: It's a good start. However, I'd personally recommend starting over. You moved all the code all over the place, when really all you needed to do was remove currentStageProvider and the Stage class. Then, rename _StudentView and _TeacherView to lack an underscore, annotate them with @RoutePage() as you did, and split them into two files. At that point, see what's erroring, and switch them to use routes instead of Riverpod. It should also be noted that the tests are failing because of how you configured the router. Retain the current setup, but have the main page be like WrapperPage, having children in the AppRouter singleton and using the AutoRouter widget to get children. Maybe use the guards list to bounce people to the right page? Another thing to keep in mind is that you can use query parameters for ViewCoinsStage.student, which will let you make that passable in navigation.

lishaduck pushed a commit that referenced this issue Nov 27, 2023
lishaduck pushed a commit that referenced this issue Dec 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dart Regards Dart code enhancement New feature or request
Projects
Status: 🔖 Ready
Development

Successfully merging a pull request may close this issue.

3 participants