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

Converting to Mojolicious::Template for outputFormat #14

Open
wants to merge 12 commits into
base: develop
Choose a base branch
from

Conversation

drdrew42
Copy link
Member

@drdrew42 drdrew42 commented Jun 6, 2023

Building off of #11 and #13, this PR converts FormatRenderedProblem and AttemptsTable over to the use of Mojolicious::Template to align with WW2's RPC calls.

Deviations from WW2

Some changes were required:

  • The renderer does not use CourseEnvironments, so this use has been stripped out.
  • The renderer does not use WebService objects, so some minor modifications were necessary here as well.
  • submitAnswers, previewAnswers, and showCorrectAnswers form button names were changed for consistency.
  • getAssetURL used CE to access language settings, they are instead passed in as an argument in place of CE
  • themes are irrelevant to standalone renderer, maybe this is something to add support for?
  • a few other tweaks were added, in support of changes from A Collection of Minor Enhancements #11 and Starting a cleanup push #13 -- particularly ability to hide the attempts table.

Updates to Functionality

  • outputFormat no longer selects one of the WebworkClient *_format.pl files as a template. The template is now templates/RPCRenderFormats/default.html.ep unless specifically requesting ptx or raw as the outputFormat
  • displayMode: PTX now supported
  • js and css assets in public/ are now processed similar to pg/htdocs, minified and version-tracked

@drdrew42 drdrew42 changed the base branch from main to develop June 6, 2023 02:03
@drdrew42 drdrew42 force-pushed the feature/mojo-templates branch from 73e3915 to 11ef91a Compare June 6, 2023 13:19
@drdrew42
Copy link
Member Author

drdrew42 commented Jun 6, 2023

Still to-do:

  • update Dockerfile
  • test on a variety of problems: images, graphTool, GeoGebra, etc...
  • LINTING
  • consider structure/content for requesting various JSON (raw vs default; with/without pg flags; etc)

Copy link
Member

@drgrice1 drgrice1 left a comment

Choose a reason for hiding this comment

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

Here are a few observations and suggestions.

You should delete the package.json and package-lock.json files in the renderer root directory, delete the public/vendor linke, and update the links in templates/columns/editorUI.html.ep to use the codemirror files in public/node_modules. Also, the package-lock.json file for the new package.json file in public should be added.

I noticed that the submit buttons ("Preview My Answers", "Submit Answers", etc.) in problems do not seem to be working with this pull request.

Are the files in lib/WebworkClient used anymore? Are keeping them for now for reference?

lib/RenderApp.pm Show resolved Hide resolved
lib/RenderApp.pm Show resolved Hide resolved
lib/WeBWorK/AttemptsTable.pm Outdated Show resolved Hide resolved
lib/WeBWorK/AttemptsTable.pm Outdated Show resolved Hide resolved
public/css/bootstrap.scss Outdated Show resolved Hide resolved
public/css/bootstrap.scss Outdated Show resolved Hide resolved
public/css/bootstrap.scss Outdated Show resolved Hide resolved
@drgrice1
Copy link
Member

drgrice1 commented Jun 9, 2023

Another thing that I noticed is that the minimized javascript and css files and the public/static-assets.json file need to be added to .gitignore.

@drdrew42
Copy link
Member Author

You should delete the package.json and package-lock.json files in the renderer root directory, delete the public/vendor link, and update the links in templates/columns/editorUI.html.ep to use the codemirror files in public/node_modules. Also, the package-lock.json file for the new package.json file in public should be added.

Done.

I noticed that the submit buttons ("Preview My Answers", "Submit Answers", etc.) in problems do not seem to be working with this pull request.

There was an issue with changes to the JSON structure, causing an error in the js that handled updating the iframe. I have resolved it (warnings are now displayed by the default template, and there is no need to capture them as I was doing before).

Are the files in lib/WebworkClient used anymore? Are keeping them for now for reference?

DELETED!

Copy link
Member

@drgrice1 drgrice1 left a comment

Choose a reason for hiding this comment

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

A few other things seem to need to be fixed.

The problem submit buttons are now submitting, but they are not submitting in the correct way. If you click on "Preview My Answers" or "Submit Answers" they both do the same thing. A preview does not just show a preview, but grades the problem.

It would be good to add the "Show Correct Answers" button back to the standalone renderer. That is a useful tool for editing problems.

lib/WeBWorK/FormatRenderedProblem.pm Show resolved Hide resolved
templates/columns/editorUI.html.ep Outdated Show resolved Hide resolved
@drdrew42
Copy link
Member Author

The problem submit buttons are now submitting, but they are not submitting in the correct way. If you click on "Preview My Answers" or "Submit Answers" they both do the same thing. A preview does not just show a preview, but grades the problem.

Yep, I see that. I'll handle this tonight.

It would be good to add the "Show Correct Answers" button back to the standalone renderer. That is a useful tool for editing problems.

Does this seem reasonable to include by default whenever we have isInstructor true?

@drgrice1
Copy link
Member

It would be good to add the "Show Correct Answers" button back to the standalone renderer. That is a useful tool for editing problems.

Does this seem reasonable to include by default whenever we have isInstructor true?

That seems right to me. That is consistent with how it works in webwork2 for regular homework sets.

@drgrice1
Copy link
Member

I noticed there is an issue with the submithelper.js file. It is my mistake from earlier changes. There should be a (); at the end of the last line in that file. So the file should be

(() => {
    ...
})();

Since that is missing the file contains an anonymous function definition, and the function is never called. As a result the minification completely removes everything, and the minimized file is actually an empty file.

@drdrew42 drdrew42 force-pushed the feature/mojo-templates branch from 7daaa04 to f1ad558 Compare June 14, 2023 01:59
@drdrew42
Copy link
Member Author

Okay, other than the fixes for hosting the renderer with an extended root URL, I think this is ready. I'm going to put all of those fixes in another PR.

Copy link
Member

@drgrice1 drgrice1 left a comment

Choose a reason for hiding this comment

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

Lets plan on merging this soon so that we can move on.

There is one minor clean up detail that was missed in my earlier comments. Please delete the package-lock.json file in the root directory of the repository. That isn't needed anymore.

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.

2 participants