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

WeBWorK: static preview for 2.16+ #1333

Merged
merged 1 commit into from
Jul 27, 2020
Merged

Conversation

Alex-Jordan
Copy link
Contributor

This is not ready to merge, but it is ready for some testing and feedback. I'm going to be offline the next few days, and I wanted to get some eyes on this to see what you think. The only testing I did is making the sample chapter HTML with the webwork-ptx server and it seems OK. I didn't check a diff though. And then making the sample chapter HTML with the webwork-dev server and it seems OK too.

I haven't considered accessibility, but I will before this PR is closed and merged. There needs to be aria-live somewhere.

The Runestone stuff is on, which I think should not be the case for the final PR here. It makes some console errors when the book is not actually in Runestone. I intend for this PR to just be about the static preview -> live WW mode for HTML output. Not anything having to do with Runestone. When this one is squared away, I'm finally back to Runestone (@bnmnetp).

Also it loads a js file from my school's faculty server. That will need to evolve and go into JS_Core. And there will need to be some better styling.

A future commit (in a future PR) should ditch the traditional WW results table in favor of something better. Like marks of some sort right where the answer blanks are. But done accessibly.

@rbeezer
Copy link
Collaborator

rbeezer commented Jul 17, 2020

Really nice. I love the way the correct answer "goes into" the answer blank.

  • You keep checking for server-url and server-data. Does it make sense to write one marker (server class) into representations and set a global variable when the representations are read in? Values like pre-2.16, 2.16-post, or similar? Perhaps version the representations (no mention of server number).
  • You use a value-of to strip off /webwork2 for a server URl (right?). Set it in the representations? If not, I have a further comment.
  • I think I saw a @host. Might it be @data-host? No further investigation.
  • You can now remove definition of $b-webwork-inline-randomize?

@Alex-Jordan
Copy link
Contributor Author

Thanks for reviewing things. I may pick back up on this tomorrow.

You keep checking for server-url and server-data. Does it make sense to write one marker (server class) into representations and set a global variable when the representations are read in? Values like pre-2.16, 2.16-post, or similar? Perhaps version the representations (no mention of server number).

Maybe, would that be your preference? The first time was the place where it chooses whether to write a div or an iframe, and I like how direct that was. Like, is there a server-url? If yes, that's what I need to write down now. The other instances came later.

You use a value-of to strip off /webwork2 for a server URl (right?). Set it in the representations? If not, I have a further comment.
I think I saw a @host. Might it be @data-host? No further investigation.

Right now the server-data has an @host, with a value like "https://webwork-dev.aimath.org/webwork2/". That's in PTX, not in HTML, so no strict need for "data-". Or are you saying "data-host" is a preferable PTX attribute name?

Yes, it could just become "https://webwork-dev.aimath.org" and the "/webwork2" can be added where needed.

You can now remove definition of $b-webwork-inline-randomize?

Maybe. What if you have a 2.15- server and you want to keep using this? Like Andrew R's thing? If we drop this experimental undocumented thing, we would need to tell Andrew to use the webwork-dev server until real 2.16 servers exist. And I can't promise it is usable 24 hours a day, 7 days a week.

@Alex-Jordan
Copy link
Contributor Author

OK, now that I look things over, I think it is almost ready for more serious merge consideration.

  • There are the small items to resolve mentioned in the last two posts.

  • I put an aria-live attribute on the div. I think that's all that needs doing for now wrt accessibility. I did email Michael Cantino to get his thoughts on this. (You may remember him from the PCC PTX boot camp.)

  • A WW 2.16+ server can still deliver the problems in the iframe way. Should we allow this as a publisher option, just because we can?

  • @davidfarmer can you take a quick look at this example? What would you need written into the HTML in order to style the "Make Interactive" button inside one of the exercises? Does it need a class or would you select it somehow based on its ancestors?

I think that would be all for this PR, unless you see something more. The remaining work is in CSS_core and JS_core.

@Alex-Jordan
Copy link
Contributor Author

You can now remove definition of $b-webwork-inline-randomize?

I see what you mean. It was an accident that I deleted the lines that make that button. For now I put it back, for the sake of Andrew's project at least.

@rbeezer
Copy link
Collaborator

rbeezer commented Jul 17, 2020

  • I really like the pattern:
<xsl:if test="foo:>
  <xsl:value-of select="foo"/>
</xsl:if>

But I also really like the idea of an (internal) version number on the representations file, independent of the server numbering. Just 1, 2, ... And the server name explicitly in that file. Your call.

  • Understood on $b-webwork-inline-randomize.
  • I just thought there might have been a @host mis-match. Sorry for the noise.
  • If you keep it,
<xsl:value-of select="substring-before($document-root//webwork-reps/server-url[1], '/webwork2')" />

trades on the fact that value-of only interprets the first node. I've never quite understood why and never consciously used that. But in XSLT 2.0 I think this is done differently. Would it be clearer at upgrade-time, and identical, if you went

<xsl:value-of select="substring-before($document-root//webwork-reps[1]/server-url[1], '/webwork2')" />

Having written that, now I really am in favor of not discovering the server this way. ;-)

One general UI question. Is there another way to make it obvious that the WW icon screams "Click me to go Live!". Yes, students will know quickly, but it is too nice a feature to be lost on casual browsers and potential adopters. Oh, now I see the "Make Interactive" button. No good reason, but I want to put it below the problem - maybe with very light styling (thin border, shaded background, drop shadow) around the problem (not introduction). Maybe with the button inside, or tightly bound to the border? Just designing out loud. ;-)

This is going to be very nice...

@rbeezer
Copy link
Collaborator

rbeezer commented Jul 17, 2020

Missed this one:

A WW 2.16+ server can still deliver the problems in the iframe way. Should we allow this as a publisher option, just because we can?

Publisher switches are expensive and a PITA. This is so much better, I can't see a reason to keep the iframe way. Definite improvements, without authors' invovement, should always just happen. In tnhis case, itmight even just cause minor confusion.

@Alex-Jordan
Copy link
Contributor Author

Got it. Will factor all this in, then lots of testing, then a force push.

Something else though. Delivering through an iframe, if it is a server problem that has a coded hint/solution, the user sees those things no matter what the publisher hint/solution settings were. It's out of our control.

Delivering through the div, our own javascript will intercept the rendered problem and has the option of omitting hint/solution from the rendering the user sees. Do we want to use this to respect those publisher settings now that we can?

  1. A simple approach is to put boolean data attributes in the HTML div that the js can use to to decide whether or not hint/solutions show. This is so transparent that a student user examining the HTML source could easily hack their way to seeing the solution.

  2. More complicated approach. Within the WW server, some sort of unique hash gets attached to each solution and to each hint. It's one thing for the ptx output format, and something else for json output, but there's a trapdoor function from the ptx version to the json version. And pretext.py retrieves the ptx version for each problem and they get written into the HTML div as data attributes when the publisher settings are such that users are supposed to see hint/solution. Later, our js calls the server to ask for that hash (json version), and only when it matches the trapdoor output from what is in the div data attributes does the js provide the hint/solution.

Option 1 is easy to do. Do we care that it is easy to hack?

Option 2 can also be hacked if someone researches the WW html2xml delivery method and/or peers into our js. But it's significantly more obfuscation. With 2, the technical details of getting it working on the WW server side feel like it may take many hours of experimentation. Is this worth doing?

Is there a 3?

@rbeezer
Copy link
Collaborator

rbeezer commented Jul 17, 2020

Do we care that it is easy to hack?

This is a concern of mine. Not huge, but I think it should not be trivial to discover the answers.

Is there a 3?

ROT13? Seriously, what if the javascript (and WW server?) used a symmetric encyption algorithm? I'm not suggesting managing keys like it is DRM - just enough obsfucation that a user would need to discover the JS in use, get a shared key from somewhere, write a function... In the end, an enterprising student can spin up their own server, no? At least we now have a good scheme for PTX solutions to be kept private while still part of an open-source project.

Having said all that - can you get (1) working and come back to (2) in time for 2.16? That'd let Brad experiment sooner.

I'm thinking one-third of FCLA reading questions should become their (old, existing) WW-randomized, auto-graded versions on Runestone...

@Alex-Jordan
Copy link
Contributor Author

I think I will follow that suggestion and just do (1) for now.

I think I don't want to do (3) because it would probably just be a stopgap until I could do (2), and I don't want to make more than one PR to webwork about it.

Got some accessibility feedback from PCC staff today. Will act on that too.

@Alex-Jordan
Copy link
Contributor Author

What if I do (1) (for now) with a little ROT13 obfuscation? Like instead of the div having data-showSolution="yes" it has data-fubjFbyhgvba="lrf"?

@rbeezer
Copy link
Collaborator

rbeezer commented Jul 18, 2020 via email

@Alex-Jordan
Copy link
Contributor Author

Option 4: The static preview is going to show hint/answer/solution knowls depending on the publisher settings in the usual way. So these are written into the HTML. The JS could peek to see if these knowls are there and infer whether it should show these things after the problem becomes live. And then on subsequent reloads (say from hitting the check answers or randomize button) it could similarly peek into the right places in the DOM to infer what is revealed on the reload.

Still option 1 for the short term, catching up to where Brad can do more work.

@Alex-Jordan
Copy link
Contributor Author

The representations file has this structure:

webwork-representations
  webwork-reps
  webwork-reps
  ...

It makes logical sense to put a version on the webwork-representations rather than each of the webwork-reps. But then how would the HTML extraction access that through the assembly phase?

@rbeezer
Copy link
Collaborator

rbeezer commented Jul 19, 2020 via email

@Alex-Jordan
Copy link
Contributor Author

Right, I'm thinking the top-level element of the representations file has a @version.

But my question is how does pretext-html.xsl access that? I feel like pretext-assembly.xsl needs to see the @version in the representations file and insert something into assembled source at the docinfo level. How would I do that?

@rbeezer
Copy link
Collaborator

rbeezer commented Jul 20, 2020 via email

@rbeezer
Copy link
Collaborator

rbeezer commented Jul 20, 2020 via email

@Alex-Jordan
Copy link
Contributor Author

If we are OK with that redundant option, we could just put the version inside each webwork-reps instead of in the root webwork-representation. Then assembly would not be relevant. This is easy to do, but seems "wrong".

Putting the WW version as an attribute in the root pretext element also seems wrong to me. Again, it would get the job done though.

What seems "right" to me is if the assembly added a child to docinfo. Something like webwork-info with @version. Is that possible with assembly?

@rbeezer
Copy link
Collaborator

rbeezer commented Jul 20, 2020

Yes, docinfo might make the most sense. But I think an attribute on each webwork-reps is more practical. Localized in the "assembly" phase and localized when needed. And it doesn't feel "wrong" to me -- it is a property of what is contained.

@Alex-Jordan
Copy link
Contributor Author

This is ready for review. It requires a .js file that I put in the appropriate place on the aimath server. I'll open a PR for that soon to JS_core.

I tested with webwork-ptx (WW 2.15) comparing diffs on the representations file and the html output. I get no unexpected differences at this point.

Then I tested with webwork-dev (WW 2.16/develop) comparing diffs on the representations file and the html output. I get no unexpected differences at this point either. And with that .js file in place, the result seems to work for me.

This also includes the Runestone manifest bit. If you add the right thing to the publisher file, that also should be working. (It was working earlier when Brad did some testing.)

@Alex-Jordan
Copy link
Contributor Author

Once this is reviewed and merged, I will try to build ORCCA HTML with a RS manifest for Brad to play with.

Oh, I know there is discussion of this and that in the forum. Like putting "Correct!" to the right of the answer blank. That's all fine, and is controlled by pretext-webwork.js. So it's independent fro this. Same with styling, which I haven't worked on yet.

@rbeezer rbeezer merged commit 0776d6c into PreTeXtBook:dev Jul 27, 2020
@rbeezer
Copy link
Collaborator

rbeezer commented Jul 27, 2020

Looking good. The cylinder graphic responds to the "Randomize" button! Not sure why that surprises me. ;-)

The one source problem got split into two paragraphs. Then the LaTeX changed as:

 \begin{inlineexercise}{}{g:exercise:idp89}%
-There is math in each option for this question. Which expression is not a polynomial? \par
+There is math in each option for this question. Which expression is not a polynomial?%
+\par
+\par
 \begin{itemize}[label=$\odot$,leftmargin=3em,]

Is there one too many \par in there? Which step of the way might be responsible? I saw no change in the PG rpblem sets. Not critical, just one observation.

PR tip: Before/after testing is easier for me if incidental source changes (i.e. sample-chapter) are on their own commit, and they precede code changes. Then I can rollback with only code changes. And it is hard for me to split myself since I lose author information and then have to add that back manually.

I have pretext.py changes on a branch. Fingers crossed - I don't think they intersect. My problem anyway...

@rbeezer
Copy link
Collaborator

rbeezer commented Jul 27, 2020

I have pretext.py changes on a branch. Fingers crossed.

Clear. Phew!

@Alex-Jordan
Copy link
Contributor Author

The one source problem got split into two paragraphs.

With that one, the HTML was not starting a new paragraph for the radio buttons, so the first button was coming out in a bad place. I edited source to start a new paragraph. Now I'm unsure if maybe radio buttons should just be expected to start a new paragraph? Either my edit is fine and the tex translation needs to lose a \par or my edit was unnecessary and HTML translation need to know to move to a new paragraph before radio buttons. What do you think?

PR tip: Before/after testing is easier for me if incidental source changes (i.e. sample-chapter) are on their own commit, and they precede code changes. Then I can rollback with only code changes. And it is hard for me to split myself since I lose author information and then have to add that back manually.

Got it, I will try to remember. I normally would not do such edits at the same time, but here each thing appeared to be a bug/issue with the JS rendering of the problem, and cleaning it in source made it clear it was not a JS issue. But I can do such things in separate commits.

Clear. Phew!

Phew! 👍

@rbeezer
Copy link
Collaborator

rbeezer commented Jul 27, 2020

What do you think?

Go back to the source, as an author. Should an author need to know a radio-button problem needs a new paragraph? And only for that purpose? Or should they be "allowed" to plop down the "answer mechanism" in mid-sentence just like for a var and they'll get the presentation they might expect?

@rbeezer
Copy link
Collaborator

rbeezer commented Jul 27, 2020

The

<xsl:variable name="webwork-domain">

is throwing its otherwise all the time for an HTML conversion without WW. Can you condition it on the presence of any webwork-reps at all? Probably should do the same for the "version" variable just preceding.

@rbeezer
Copy link
Collaborator

rbeezer commented Jul 27, 2020

You could set the version to an empty string when there is no WW, then use that as a flag later.

@rbeezer
Copy link
Collaborator

rbeezer commented Jul 28, 2020

Last one - maybe. -html has $b-has-webwork-reps (so does -latex, they should be consolidated). That should be used, i think.

@Alex-Jordan Alex-Jordan deleted the wwnoiframe branch September 14, 2023 03:25
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.

None yet

2 participants