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

Make evaluation creation a full stepper #3080

Merged
merged 19 commits into from
Sep 10, 2021
Merged

Conversation

chvp
Copy link
Member

@chvp chvp commented Sep 9, 2021

Also change the edit page to lead back to a stepper.

Closes #2525, closes #3037.

@chvp chvp added the feature New feature or request label Sep 9, 2021
@chvp chvp force-pushed the feature/full-evaluation-stepper branch 2 times, most recently from fb99859 to e5742aa Compare September 9, 2021 10:07
Also change the edit page to lead back to a stepper.
@chvp chvp marked this pull request as ready for review September 9, 2021 10:08
@chvp chvp requested a review from a team as a code owner September 9, 2021 10:08
@chvp chvp requested review from bmesuere and niknetniko and removed request for a team September 9, 2021 10:08
@bmesuere bmesuere added the deploy mestra Request a deployment on mestra label Sep 9, 2021
@bmesuere bmesuere temporarily deployed to mestra September 9, 2021 10:49 Inactive
@github-actions github-actions bot removed the deploy mestra Request a deployment on mestra label Sep 9, 2021
Copy link
Member

@bmesuere bmesuere left a comment

Choose a reason for hiding this comment

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

👍

The code looks good. Functionally, I have the following remarks:

  • Step 1 is about selecting a deadline but this is not super clear. Maybe rename this to pick/choose deadline? Can we add the overview and callout to a card above step 1? The info below the deadline picker can then be promoted to a callout to the right of the input.
  • If you go to step 2, the label for step 3 is already visible, but if you are at step 1, the labels for 2 and 3 aren't shown yet.
  • The list of users in step 2 feels long, especially because you need to scroll to the bottom to click next. Maybe reduce the number of users on a page to 15 or 20?
  • There's a lot of white space above the filter bar. Maybe add a h-border between the two sections would help
  • If I clicked the right preset, users in the table were selected, but the search bar progress kept spinning. The console showed an error:
Uncaught (in promise) TypeError: window.dodona.initCheckboxes is not a function
    at eval (eval at <anonymous> (index.js:101), <anonymous>:3:15)
    at index.js:101
  • After selecting users, the top of the page reads "Je hebt op dit moment x gebruikers geselecteerd. Alle gebruikers verwijderen". Maybe use deselect instead of remove?
  • It would be nice to be able to quickly see all selected users but this might be for a future PR
  • When a step is done, the chosen value isn't shown next to the step title (similar to how we do this for the course copy). It could list the chosen deadline and the number of users selected.
  • The page has no title, but this could be fixed with my first remark

@chvp chvp force-pushed the feature/full-evaluation-stepper branch from bb867a7 to e289434 Compare September 9, 2021 12:05
@chvp
Copy link
Member Author

chvp commented Sep 9, 2021

Also fixes #3037 now.

@chvp chvp requested a review from bmesuere September 9, 2021 12:50
Copy link
Member

@niknetniko niknetniko left a comment

Choose a reason for hiding this comment

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

Looks good, some minor stuff I've noticed:

  • When step 2 & 3 are not clickable, the mouse pointer still is the little hand, like when hovering on links
  • When adding a score item to all exercises, the max score in the description isn't updated.
  • When clicking on step 1, the selection in step 2 & 3 are cleared, while when clicking step 2, step 3 isn't cleared. I'm not sure if this is intentional or not.
  • If you don't select any users and click though, you go to the edit page, which has step 2 & 3. I don't think it's 100% clear why (perhaps a message "you need to select users" or something would fix this)

@chvp chvp requested a review from niknetniko September 9, 2021 15:15
@bmesuere bmesuere added the deploy mestra Request a deployment on mestra label Sep 9, 2021
@bmesuere bmesuere temporarily deployed to mestra September 9, 2021 15:35 Inactive
@github-actions github-actions bot removed the deploy mestra Request a deployment on mestra label Sep 9, 2021
Copy link
Member

@bmesuere bmesuere left a comment

Choose a reason for hiding this comment

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

Looks really good!

On the edit page, it is not super clear that all edits are saved directly. Especially because there is a "to evaluation" button at the bottom. Maybe add a message somewhere in the edit view like "all changes are saved immediately" and tweak the button text to "Back to evaluation"?

@chvp chvp merged commit f890c84 into develop Sep 10, 2021
@chvp chvp deleted the feature/full-evaluation-stepper branch September 10, 2021 07:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Issues with manual selection of students for evaluation Use stepper component in evaluation wizard
3 participants