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

Add a third optional argument to ADD_JS_FILE to add script tag attributes (defer, async, etc) #576

Merged
merged 3 commits into from
May 27, 2021

Conversation

drgrice1
Copy link
Member

@drgrice1 drgrice1 commented May 21, 2021

Add a third optional argument to ADD_JS_FILE. This argument is a hash reference of attribute value pairs. These attributes will be added to the script tag that is added to the page. The code for adding these attributes to the html that is output is added in a corresponding pull request to webwork2 (see openwebwork/webwork2#1374).

The usage of this argument is documented in the POD for the method. Its usage is also demonstrated in the paserGraphTool.pl macro.

Also, fix a bug that prevented the optional second argument of ADD_CSS_FILE from working.

reference of attribute value pairs.  These attributes will be added to
the script tag that is added to the page.  This is added in a
corresponding pull request to webwork2.

The usage of this argument is documented in the POD for the method.  It
usage is also demonstrated in the paserGraphTool.pl macro (see below).

Defer loading of the jsxgraphcore.js and graphtool.min.js javascript
files.
@drgrice1 drgrice1 force-pushed the add_js_attribs_and_gt_defer branch from 4597ba4 to f5600d8 Compare May 21, 2021 11:05
@pstaabp
Copy link
Member

pstaabp commented May 26, 2021

This looks like this is mainly for parserGraphTool. I can test some problems with this, but what should I look for?

@drgrice1
Copy link
Member Author

Actually the graph tool macro doesn't really need it. It can use it though. I just used that as an example.

The point of this pull request is really about future proofing by adding versatility.

@pstaabp
Copy link
Member

pstaabp commented May 26, 2021

Reading through the docs and code, it seems that JSXGraphOptions field of the GraphTool uses it. I got the bounding box and ticks changed via this field.

Actually the graph tool macro doesn't really need it. It can use it though. I just used that as an example.

The point of this pull request is really about future proofing by adding versatility.

Is there anything using the 3rd argument. If not and just future-proofing, I feel safe to pull in.

@drgrice1
Copy link
Member Author

The only thing to test is that problems using the macros that use the ADD_JS_FILE method still work. Those are graph tool problems and Geogebra applet problems mainly at this point.

@pstaabp
Copy link
Member

pstaabp commented May 26, 2021

The graphTool problems work fine. Looks like the geogebra problems that I found are java applets. Are we still supporting those?

I found some CUNY geogebra problems and those look good.

@pstaabp
Copy link
Member

pstaabp commented May 26, 2021

I think we can merge unless we want another set of eyes.

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.

I did not have time to test, but it looks good and @pstaab reported testing.

macros/PG.pl Outdated Show resolved Hide resolved
@pstaabp pstaabp merged commit 5ae626b into openwebwork:PG-2.16 May 27, 2021
drgrice1 pushed a commit that referenced this pull request May 27, 2021
Add a third optional argument to ADD_JS_FILE to add script tag attributes (defer, async, etc)
@drgrice1 drgrice1 deleted the add_js_attribs_and_gt_defer branch May 27, 2021 17:01
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.

3 participants