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

Move a student to another class #16593

Merged
merged 19 commits into from
Dec 18, 2024
Merged

Conversation

yafred
Copy link
Contributor

@yafred yafred commented Dec 14, 2024

This PR starts addressing a concern expressed in this issue #16562

Before starting to search for the best solution to perform batch moves, there is a need for a 'move student' feature

Moving a student means:

  • Creating a new entry in the target class (linking to the same account)
  • Delete the entry in the source class

UX:

  • Click on a student
  • Click on MOVE
  • Click on the target class
  • Confirm
  • Back to source class

image

image

image

image

I'm looking forward to your feedback

@yafred yafred marked this pull request as ready for review December 17, 2024 10:03
@@ -217,6 +217,13 @@ final class StudentFormUi(helpers: Helpers, clasUi: ClasUi, studentUi: StudentUi
cls := "button button-empty button-red",
title := trans.clas.closeDesc1.txt()
)(trans.clas.closeStudent())
),
(!s.student.managed).option(
Copy link
Collaborator

Choose a reason for hiding this comment

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

why this check?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I coded the move for the managed student first and then I realized it was making sense for the non managed student too.

The views of managed and non managed students are a bit different.
When you click on a managed student in the list, you see 2 sets of actions

image
image

When you click on a non managed student you only have the first one
So I had a choice:

  • Overloading the first set of actions with the move (keeping consistency between the 2 types of students)
  • Do it one click later for the non managed in the edit form (but hiding it for the managed student because it was useless then)

So if you have a strong opinion about where it should be for this PR, I am happy to adapt.

Copy link
Collaborator

@ornicar ornicar left a comment

Choose a reason for hiding this comment

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

nice work, almost there

modules/clas/src/main/ui/StudentFormUi.scala Outdated Show resolved Hide resolved
modules/clas/src/main/ui/StudentUi.scala Outdated Show resolved Hide resolved
@ornicar ornicar merged commit 2ade9c1 into lichess-org:master Dec 18, 2024
5 checks passed
@yafred yafred deleted the class-move-student branch December 18, 2024 17:26
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.

2 participants