Skip to content

Use JavaScript to show/hide fields when switching assignment_type. - #3222

Open
somiaj wants to merge 1 commit into
openwebwork:developfrom
somiaj:js-switch-set-type
Open

somiaj wants to merge 1 commit into
openwebwork:developfrom
somiaj:js-switch-set-type

Conversation

@somiaj

@somiaj somiaj commented Sep 27, 2026

Copy link
Copy Markdown
Contributor

When selecting the assignment_type on the ProblemSetDetails page, use JavaScript to show/hide set fields for the appropriate assignment type. This way one can change a set's assignment type and edit the specific assignment type fields without having to save the changes first to see the fields for that type.

@somiaj

somiaj commented Sep 27, 2026

Copy link
Copy Markdown
Contributor Author

I just realized this isn't complete or might be not possible, because this only messes with set fields, and switching also changes problem fields. Making a draft for now as I think about what to do with the problem fields.

@somiaj
somiaj marked this pull request as draft September 27, 2026 21:32
@somiaj

somiaj commented Sep 28, 2026

Copy link
Copy Markdown
Contributor Author

This now shows/hides the problem fields based on set type as well, but the javascript would be a bit too complicated to update the set numbering to change to/from JITAR sets, so in that case a warning is shown to let the user they need to "Save Changes" to update the problems when changing to/from a JITAR set.

@somiaj
somiaj force-pushed the js-switch-set-type branch 2 times, most recently from 81a48f1 to 65e84bf Compare September 28, 2026 01:42
@dlglin

dlglin commented Sep 28, 2026

Copy link
Copy Markdown
Member

This is probably beyond the scope of this PR, but how about adding the ability to choose an assignment type when creating a set?

@somiaj
somiaj marked this pull request as ready for review September 28, 2026 15:59
@somiaj

somiaj commented Sep 28, 2026

Copy link
Copy Markdown
Contributor Author

Since I cannot think of a good way to deal with to/from jitar sets and updating problems to the jitar naming/nesting buttons, this is now ready for review again. It does update problem fields, and just warns users when they need to "Save Changes" to move to/from jitar sets to fully update the problems to/from the jitar naming/nesting ability.

@somiaj

somiaj commented Sep 28, 2026

Copy link
Copy Markdown
Contributor Author

@dlglin that could be a possibility, but there are multiple places a set can be created. Adding that option to the "Create" action tab in the "Sets Manager" would be doable. Though should this also be added when creating a set from the "Library Browser". Unsure if there is any other way a set can be created (outside of importing which already contains the type).

@drgrice1 drgrice1 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Here are some things I see at this point.

This pull request makes me a bit wary. It seems that there might be some dangerous things in this. Particularly when it comes to changing to or from a JITAR set, but there might be things missed for the other changes as well. How versatile and safe is the code for handling the set type change and other edits to settings that are made before a save occurs?

Comment thread lib/WeBWorK/ContentGenerator/Instructor/ProblemSetDetail.pm Outdated
Comment thread lib/WeBWorK/ContentGenerator/Instructor/ProblemSetDetail.pm Outdated
Comment thread templates/ContentGenerator/Instructor/ProblemSetDetail.html.ep Outdated
@somiaj

somiaj commented Oct 2, 2026

Copy link
Copy Markdown
Contributor Author

I think jitar sets are currently broken in develop. Create a jitar set, create some nesting, "save" that change. When the page is reloaded, the nesting is not shown anymore. But if you then try to move some nesting around there is some weird effects, and some of the problem fields are shown that shouldn't be. So it is like the nesting is kinda there but not fully. I ran into that testing this and discovered it wasn't due to any of my changes.

@somiaj

somiaj commented Oct 2, 2026

Copy link
Copy Markdown
Contributor Author

Currently save will update any provided inputs, or use a default value if the setting is empty. One of the things I ran into was some of the default test settings were actually set during the 'Save' to switch to the test, so I also made sure to set the default values in the form initially if they were undefined in the database.

With this setup all the hidden inputs are also sent as part of the request. This does mean if you change a setting, then change the type so that setting is no longer visible. That change will get sent and be updated for the hidden fields. I didn't think that was an issue, but I could go a step further and disable all inputs that are hidden, so only the visible inputs are sent as part of the post request.

In testing this more, switching to and from a jitar set will loose any changes to the problem order/nesting. I added that info to the message, and also no longer include the jitar problem fields for non jitar sets. Since switching to a jitar set would require a save anyways, and it makes no point to show them (which they would all be hidden anyways since there is no nesting).

@somiaj
somiaj force-pushed the js-switch-set-type branch from 07c3385 to bbe3306 Compare October 2, 2026 00:55
@drgrice1

drgrice1 commented Oct 2, 2026

Copy link
Copy Markdown
Member

I think jitar sets are currently broken in develop. Create a jitar set, create some nesting, "save" that change. When the page is reloaded, the nesting is not shown anymore. But if you then try to move some nesting around there is some weird effects, and some of the problem fields are shown that shouldn't be. So it is like the nesting is kinda there but not fully. I ran into that testing this and discovered it wasn't due to any of my changes.

I am not seeing any issues on the develop branch. I created a JITAR set and created nesting. When the page reloads the nesting is collapsed by default, but can be expanded. Moving things around seems to be fine. I am not seeing any weird effects or problem fields shown that shouldn't be (or not shown that should be).

@drgrice1

drgrice1 commented Oct 2, 2026

Copy link
Copy Markdown
Member

Can you clarify the issues with the JITAR sets that you are seeing?

@somiaj
somiaj force-pushed the js-switch-set-type branch from bbe3306 to a562cc4 Compare October 2, 2026 01:01
@somiaj

somiaj commented Oct 2, 2026

Copy link
Copy Markdown
Contributor Author

Goto an existing non jitar set and edit its settings. Change it to a jitar set, click "save". Got to problem 2 and 3 and indent them one, so the set now has problems 1.1 and 1.2, click "save".

After that when you go look at the problems, the nesting is gone, and problem "1.1" and "1.2" are not shown. But problem "1" shows the field attempt threashold for children, so it is like it knows it has children that are not being shown. At this point if you try to nest problem "2", it then nests the two problems below it making a "1.1", "1.2", and "1.3" appear.

@somiaj

somiaj commented Oct 2, 2026

Copy link
Copy Markdown
Contributor Author

Oh wait, it is my confusion, I didn't realize that nesting was collapsed by default. Though it does make things act a little weird if you try to nest things again.

@somiaj

somiaj commented Oct 2, 2026

Copy link
Copy Markdown
Contributor Author

Though I see the behavior, nesting the next problem then both expands the nesting and adds that problem to it. So it is working as expected, just not how "I" expected it to.

@somiaj
somiaj force-pushed the js-switch-set-type branch from a562cc4 to de7e514 Compare October 2, 2026 01:31
@somiaj

somiaj commented Oct 2, 2026

Copy link
Copy Markdown
Contributor Author

I asked claude to see if there is any potential issues in my changes with the save logic causing problems. Claude found no other issues than the ones I have already mentioned, and that is changes to fields that are hidden will be updated, but not used since they don't affect the new assignment type. Outside of any changes to the problem order being lost with switching to/from a jitar set, there isn't anything that Claude was able to spot as dangerous.

Claude did find one bug with switching to a jitar set, but that exists before my changes, seq_to_jitar_id dies if a problem id is >= 126 when converting a set to a jitar set.

@somiaj
somiaj force-pushed the js-switch-set-type branch 2 times, most recently from 95a4659 to 58f30a7 Compare October 6, 2026 18:10

@drgrice1 drgrice1 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I am going to go ahead and approve this for now. I am not sold on the behavior with switching to and from JITAR sets though.

@pstaabp pstaabp left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this working as well as it can--it's a bit awkward with JITAR sets, but I think it's a compromise.

@pstaabp

pstaabp commented Oct 16, 2026

Copy link
Copy Markdown
Member

I think I thought JITAR was broken as @somiaj did above. I don't usually use JITAR, so haven't noticed this until now.

It would be helpful to have something more prominent in the UI indicating that there are children of a given problem. I think now there is just the presence of a right chevron if there are children. And I just noticed the presence of another row in the table: Attempt Threshold for Children. I missed those and thought I had lost problems.

@somiaj

somiaj commented Oct 16, 2026

Copy link
Copy Markdown
Contributor Author

@pstaabp The UI is already over complicated on this page, and JITAR is probably rarely used, so not worth the effort IMO. I say leave it alone and just let user's get use to it if they use JITAR. Once I knew what to expect (I just expected sub-problems to be expanded by default, not collapsed), it wasn't an issue. It was just a learning curve, and I don't know of any clear way to improve it.

When selecting the `assignment_type` on the ProblemSetDetails page,
use JavaScript to show/hide set fields for the appropriate
assignment type. This way one can change a set's assignment type
and edit the specific assignment type fields without having to
save the changes first to see the fields for that type.
@somiaj
somiaj force-pushed the js-switch-set-type branch from 58f30a7 to f09af1e Compare October 17, 2026 14:37
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.

4 participants