Skip to content

Conversation

@drgrice1
Copy link
Member

@drgrice1 drgrice1 commented Apr 16, 2021

@pstaabp observed that there were improvements made to the simple_format in #1301, and suggested that those be added to the other formats as well. So this does that.

There are also a few tweaks to the simple format as well. Mainly just adding the answer blank coloration (but only if it is not an answer preview and is a submit or check answers).

This now also adds theme selection to the formats which fixes #1335.

This also now makes it possible to hide any of the submit buttons in the formats if desired. To hide the buttons add the GET or POST parameters showPreviewButton=no, showCheckAnswersButton=no, or showCorrectAnswersButton=no. The simple2 format has been removed, as this is all that it does that the simple format does not anyway.

Final addition: The standard format is deleted, and the debug format is beefed up to replace it.

@drgrice1 drgrice1 force-pushed the client-format-updates branch from 6556b4e to 546a096 Compare April 16, 2021 15:33
pull request openwebwork#1301.

There are also a few tweaks to the simple format as well.  Mainly just
adding the answer blank coloration (but only if it is not an answer
preview and is a submit or check answers).
@drgrice1 drgrice1 force-pushed the client-format-updates branch from 546a096 to bb54ee2 Compare April 16, 2021 15:34
@Alex-Jordan
Copy link
Contributor

This looks good.

@taniwallach
Copy link
Member

Just to reference @Alex-Jordan's other JSON output - it is the "raw" format from #1120

The json_format.pl could probably be revised to replace the CSS and JS sections from being HTML code to an array of what files are needed.

@mgage mgage added this to the WW 2.16 milestone May 5, 2021
the submit buttons.  They are all shown by default (as it was before).
This is implemented for the standard, simple, sticky, and json formats.
It does not apply for the others.

To hide the buttons add the GET or POST parameters

`showPreviewButton=no`,
`showCheckAnswersButton=no`, or
`showCorrectAnswersButton=no`.

The simple2 format is removed as this makes that format pointless.
@drgrice1 drgrice1 force-pushed the client-format-updates branch from 5eb8866 to 7edd635 Compare May 7, 2021 02:52
drgrice1 and others added 2 commits May 6, 2021 22:02
format and clean that up a bit.
Separate the translator warnings (that don't really exist) and the perl
warnings that come from pg that are sent on from renderProblem.
an array of URLs to be used in href, rather than sending the full HTML
code to load them. The JS loads which need the "defer" attribute were split
from head_part200 into head_part201, and one of those loads
	/webwork2_files/mathjax/es5/tex-chtml.js
should also get id="MathJax-script" in the <script> tag.
The creation of suitable HTML code, including the addition of defer and id
attributes where needed should be done on the side processing the JSON data.

Fixed the lines using $themeDir to be in single quotes so the variable
interpolation will be done properly later on, when that variable is available.
Copy link
Member

@taniwallach taniwallach left a comment

Choose a reason for hiding this comment

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

@drgrice1 - Great stuff. Tested and mostly working.

  1. The formats: simple, debug and sticky all seem to work as expected.
  2. sticky's use of local storage to save the last answers seems fine.However, I do not see colors in the results table for the stick format. I think this is worth fixing, unless it requires an inordinate amount of effort. I did not look into why it does not work now.
  3. json needs some small changes, and this is also a chance to replace the JS and CSS loading data with arrays of the addresses, rather than a block of full HTML code as recently discussed. I made a PR drgrice1#3 to the branch of thus PR with what I think needs to be changed for the json_format.

Two more minor comment - which need not hold back merging the improvements:

  • Why use no for the new settings like: showPreviewButton=no? Maybe at least allow 0 also for no?
  • For showCorrectAnswersButton maybe the default should be no although that changes the existing default behavior.

@drgrice1
Copy link
Member Author

drgrice1 commented May 7, 2021

  1. sticky's use of local storage to save the last answers seems fine.However, I do not see colors in the results table for the stick format. I think this is worth fixing, unless it requires an inordinate amount of effort. I did not look into why it does not work now.

I see what you see. I will look into it.

  1. json needs some small changes, and this is also a chance to replace the JS and CSS loading data with arrays of the addresses, rather than a block of full HTML code as recently discussed. I made a PR drgrice1#3 to the branch of thus PR with what I think needs to be changed for the json_format.

Your pull request looks good. I will merge it. More could be done here. I think that instead of using prefixed keys for the hidden inputs like hidden_input_filed_sourceFilePath, it would be nicer to use a sub object. So one key hidden_input_fields that is itself an object, and each of the inputs a sub key. It takes a little more work to set up, but is better structurally.

  • Why use no for the new settings like: showPreviewButton=no? Maybe at least allow 0 also for no?

I am fine with using '0' instead (I think). I would rather not allow multiple settings as I don't want to overly complicate the code in FormatRenderedProblem.pm.

  • For showCorrectAnswersButton maybe the default should be no although that changes the existing default behavior.

I would rather not change the existing default behavior unless there is a consensus to do so.

drgrice1 and others added 3 commits May 7, 2021 06:07
@drgrice1
Copy link
Member Author

drgrice1 commented May 7, 2021

@taniwallach: I have addressed all of the things you mentioned except the default behavior for the show correct button. I am fine with making that change, but I will let others weigh in on that idea first.

@Alex-Jordan: You probably use the html2xml formats more than most others. What do you think of defaulting to not showing the "Show Correct Answers" button.

@drgrice1 drgrice1 requested a review from taniwallach May 7, 2021 11:48
problem in webwork.  This is so that the javascript in mqedit.js (for
MathQuill answers) picks up on it and enter triggers a preview.
Also fix some errors in the localstorage.js javascript for the sticky
format.
@taniwallach
Copy link
Member

  1. json needs some small changes, and this is also a chance to replace the JS and CSS loading data with arrays of the addresses, rather than a block of full HTML code as recently discussed. I made a PR drgrice1#3 to the branch of thus PR with what I think needs to be changed for the json_format.

Your pull request looks good. I will merge it. More could be done here. I think that instead of using prefixed keys for the hidden inputs like hidden_input_filed_sourceFilePath, it would be nicer to use a sub object. So one key hidden_input_fields that is itself an object, and each of the inputs a sub key. It takes a little more work to set up, but is better structurally.

Done in drgrice1#4

  • Why use no for the new settings like: showPreviewButton=no? Maybe at least allow 0 also for no?

I am fine with using '0' instead (I think). I would rather not allow multiple settings as I don't want to overly complicate the code in FormatRenderedProblem.pm.

Thanks!

  • For showCorrectAnswersButton maybe the default should be no although that changes the existing default behavior.

I would rather not change the existing default behavior unless there is a consensus to do so.

I agree. If there is such a consensus it can be changed later.

@drgrice1 drgrice1 force-pushed the client-format-updates branch from 16a68a1 to 1963133 Compare May 7, 2021 13:36
@taniwallach taniwallach requested a review from pstaabp May 7, 2021 14:48
@taniwallach
Copy link
Member

@pstaabp - I think this is ready to go, except the possible change to the default. Could you take a look?

@taniwallach taniwallach merged commit 1c9f75b into openwebwork:WeBWorK-2.16 May 12, 2021
drgrice1 pushed a commit that referenced this pull request May 12, 2021
Update the other formats with the features added to the simple_format in #1301
@drgrice1 drgrice1 deleted the client-format-updates branch May 12, 2021 21:17
<link rel="stylesheet" type="text/css" href="/webwork2_files/themes/math4/math4.css"/>
<link rel="stylesheet" type="text/css" href="/webwork2_files/css/knowlstyle.css"/>
<link rel="stylesheet" type="text/css" href="/webwork2_files/js/apps/ImageView/imageview.css"/>
<link rel="stylesheet" href="$themeDir/math4.css"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

Something about this is not working. Here is the AIM server, now using 2.16. For normal use, things appear to be OK. But for using the simple format, $themeDir is resolving as the empty string, as you can see in the source of the page below. So the CSS and js is not loading. Here is an example page:

https://webwork-ptx.aimath.org/webwork2/html2xml?courseID=anonymous&userID=anonymous&password=anonymous&course_password=anonymous&answersSubmitted=0&displayMode=MathJax&outputformat=simple&problemSeed=6&problemSource=RE9DVU1FTlQoKTsKbG9hZE1hY3JvcygiUEdzdGFuZGFyZC5wbCIsIk1hdGhPYmplY3RzLnBsIiwiUEdNTC5wbCIsIkFuc3dlckZvcm1hdEhlbHAucGwiLCJQR2NvdXJzZS5wbCIsKTtURVhUKGJlZ2lucHJvYmxlbSgpKTsKQ29udGV4dCgnTnVtZXJpYycpOyRhID0gcmFuZG9tKDEsIDksIDEpOwokYiA9IHJhbmRvbSgxLCA5LCAxKTsKJGMgPSBDb21wdXRlKCRhICsgJGIpOwoKQkVHSU5fUEdNTApDb21wdXRlIHRoZSBzdW0gb2YgW2BbJGFdYF0gYW5kIFtgWyRiXVx0ZXh0ezp9YF0KCltgWyRhXSArIFskYl0gPWBdIFtfXXskY317NX0gW0BBbnN3ZXJGb3JtYXRIZWxwKCdudW1iZXJzJylAXSoKCgpFTkRfUEdNTAoKQkVHSU5fUEdNTF9TT0xVVElPTgpbYFskYV0gKyBbJGJdID0gWyRjXVx0ZXh0ey59YF0KCgpFTkRfUEdNTF9TT0xVVElPTgoKRU5ERE9DVU1FTlQoKTs%3D

I've searched through the config files and I can't find something that should fix this. $themeDir is defined with my $themeDir = "$ce->{webworkURLs}{htdocs}/themes/$theme"; in FormatRenderedProblem.pm. And in defaults.conf, I find $webworkURLs{htdocs} = "$webwork_htdocs_url";. And in site.conf I find $webwork_htdocs_url = "/webwork2_files";. But I have trouble seeing how $themeDir resolves as the empty string.

Copy link
Contributor

Choose a reason for hiding this comment

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

I copied the line
my $themeDir = "$ce->{webworkURLs}{htdocs}/themes/$theme";
in FormatRenderedProblem.pl down to immediately before the interpolation at line 357, and now things work. So something is not working with its original location at line 130.

@drgrice1
Copy link
Member Author

I am not able to reproduce this. Even going to the link that you posted seems to work. This may be because you have made the change you mention on your server. Although, I also can't see how making that change would fix anything. That variable depends on the $ce variable which is defined above line 130.

@Alex-Jordan
Copy link
Contributor

Yes, it fixed once I copied line 130 to line 357, but that is all I did.

I will use a different server to try to cleanly reproduce it.

@Alex-Jordan
Copy link
Contributor

I'm not able to reproduce it on the next server I tried. And I deleted the line I pasted in the AIM server, restarted apache, cleared cache, and I can't reproduce it there now either.

Note that the issue as I understood it is not about $ce. Even if $ce were undefined, the line

my $themeDir = "$ce->{webworkURLs}{htdocs}/themes/$theme";

should leave $themeDir as a nonempty string with "themes" inside it. But what was happening was in the HTML source for the page like that URL I posted, instead of lines like

<link rel="stylesheet" href="/webwork2_files/themes/math4/math4.css"/>

I was getting lines like

<link rel="stylesheet" href="/math4.css"/>

OK, well nevermind then. I'm never sure if caching somewhere is messing with me. If it comes up again, I will carefully try to make it happen in a reproducible way on some other development server.

@drgrice1
Copy link
Member Author

There is certainly something odd that occurred.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants