Various improvements to the teacher page, incl. some inline documentation #549
Conversation
- optionally allow cancelling (for partition view) - focus the input field on display - validate on "enter" keypress
this still affects students it has not been assigned to
the interface is still terrible, and the doc not in the best format, but this is so much better than nothing ;)
following the previous patches. Note that the teacher page doc content is not yet translated; contributions welcome ;)
|
Hm I got some gettext issues on that last patch, otherwise should be good for review |
| @@ -211,7 +211,7 @@ let rec teacher_tab token _select _params () = | |||
| in | |||
| let open_partition_ () = | |||
| Lwt.async (fun () -> | |||
| ask_string ~title:"Choose a function name" | |||
| ask_string ~title:"Partitioning of student solutions" | |||
There was a problem hiding this comment.
I am not sure that's a valid patch: we really want to ask a function name here.
There was a problem hiding this comment.
The question is below, this is only the title of the dialog. My reason for this change is that you get there by ctrl-click on an exercise (i.e. not an explicit button) and previously you may get pretty confused as to what this was about.
There was a problem hiding this comment.
Hi @AltGr I tested all the features you implemented in this PR, looks really great!
There's only one issue I spotted (I use firefox-esr on Debian 11):
- 03669eb → the
validate on "enter" keypressdoesn't work for me in the"Partitioning of student solutions"dialog, the"enter"key is a no-op; - while for c341fca → the
"enter"key works here.
Can you reproduce the issue?
Apart from that, the PR can be merged as is I think (using a regular merge, NOT using a squash).
|
Ah @erikmd it's weird since it's the same code being called… I re-tested and my guess is that it's probably caused by the popup blocker (I know the browsers limit certain triggers to specific events, like you cannot turn the page fullscreen without user interaction, so maybe a click is required in this case). Here is what I get upon pressing the Enter key: |
|
Thanks @AltGr! Note: don't be surprise that I assign the PR to you after-the-fact; I just follow the convention that PR assignee = merger. |
No description provided.