button to randomize individual seeds on set detail page for one user #2271
Conversation
Alex-Jordan
force-pushed
the
randomize-seeds
branch
from
10921e4 to
5c074ca
Compare
|
The button itself looks fine. I added a pull request to this branch with suggestions for code improvement. Most importantly the inline javascript is moved out of the Perl module, and into the |
|
Now with a button to randomize all seeds for one user for one set. I strongly suspect the javascript can be improved upon. One thing for sure is it looks up the checkbox state over and over within the loop. I couldn't work out how to do that better without getting js errors. |
Alex-Jordan
force-pushed
the
randomize-seeds
branch
2 times, most recently
from
f1ee4d3 to
6691680
Compare
|
Rebased, including with the correction mentioned here. This is as far as I plan to take this PR. The other tools I want to add for randomization can come later. |
There was a problem hiding this comment.
Looks good for now. Lets get this in so @Alex-Jordan can continue with other related changes as planned.
Inline javascript is bad form. There is already a javascript file dedicated to the ProblemSetDetail.pm page, so put the javascript there instead. The return value of the Mojolicious::Plugin::TagHelpers `tag` method is already a Mojo::ByteStream object. The issue that causes the display to not work is that the contents of the `div` are not, since they are just bare strings concatentated together, and so they are html escaped. So use a Mojo::Collection that is joined by the empty string. That also returns a Mojo::ByteStream object, and so it is not html escaped. This is also consistent with how I have done this sort of thing elsewhere in the code. Also remove what appears to be an errant `warn` statement.
Alex-Jordan
force-pushed
the
randomize-seeds
branch
from
6691680 to
b0dfa9b
Compare
This is a first stab at making it easier to randomize seeds, as discussed in #1151.
In a Set Detail page for one user:
type='number'to give a quick way to shift a seed by 1 at a time, which I find helpful if there might be a need to revert to some previous seed.After any changes, the Save button still needs to be used to submit the form.
If this is on the right track, I will add a button somewhere at the top to randomize all of the seeds at once. If it is not too much clutter, there will be an option to only randomize those seeds with status less than 1.
A future PR will have a button on the "Set Detail page for multiple users" that randomizes seeds for all users and goes ahead with the action, reloading the page. And this PR will add a tool to Instructor Tools for doing the same thing, possible with multiple sets too.