-
-
Notifications
You must be signed in to change notification settings - Fork 166
refactor hardcopy themes #2099
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
refactor hardcopy themes #2099
Conversation
|
Here are three themes based on the You will need to change the extension of these files to |
1751f5d to
5844065
Compare
|
Overall this looks great. However, if I add your custom files above, they are tracked. How about a Also, is there a mechanism for instructors to do this for a particular course. If we update |
|
I'm making some enhancements following today's meeting. So it may be best not to review this quite yet. |
|
OK, I made some changes that do some of what @pstaabp suggested. Now you can get on this branch (and the corresponding pg branch) and in a course:
|
fc0d107 to
43f19d2
Compare
drgrice1
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't delved too deeply into this yet, but here are some things that I see already.
I don't like that when a hardcopy is generated in the PG problem editor via HardcopyRenderedProblem.pm, there is now a horizontal rule at the top of the page.
The \firstpageheader from the theme is now written after the set header. This means that it is impossible for the set header to override that. This is a show stopper for me. I don't want the default headers.
I am not entirely fond of having TeX in XML, but I can get over that I suppose.
9269ed0 to
5aedc26
Compare
|
I changed the minor things.
That rule is part of the theme. It seems wrong to let certain things appear from the theme (like let the twoColumn theme use two columns and have a vertical line between them) but not let other things from the theme appear. Could you render using the Basic theme or the Empty theme instead? Would it help to have a "Basic Two Columns" theme? Or a new theme that has the things you currently use, including a lack of a vertical rule? Would it help to allow for a different default theme for the PGEditor (eg Basic) than the default for full hardcopy production (eg oneColumn)?
So, the point of much of this is that you could easily have your own hardcopy theme that does not have a If you can send me a sample tex bundle for a homework set that is in a style you like, I could add that as a hardcopy theme that comes with the distribution. I don't even care to keep the current oneColumn and twoColumn exactly as they are. if yours is better, I'd change those to what you are using in production. Already this evening I realized that I need to change something about the |
Thanks.
This must not insist on an instructor writing a new theme to obtain this. I can not approve this pull request in this way. As I said, this is a show stopper. |
drgrice1
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are some more issues that I see.
|
On the issue of the |
|
There is another issue that I just noticed. The problem number is now being displayed as "Problem1", "Problem2", ..., instead of "Problem 1", "Problem 2", ... |
Which theme did you see this with? These should all be written with |
|
I see it with both the one and two column theme. |
|
In both themes this is added as |
|
Got it. My eyes/brain turned the "Problem 1" you wrote into "Page 1" and that is what I was looking at. Sorry! |
|
You also need to update the |
I accept that I haven't persuaded you yet on this principle, but the PG set header file can now be divorced from styling the PDF. And I feel that it should be divorced, and that will make some things better in the long run. But your method to override things in the theme using the PG header runs contrary to that principle. What happens when you override Up to now it has been more work to make a new theme than to make a new PG header file. With this PR, they are a lot closer. Something I can still improve on is to make the Editor capable of editing a theme file like how it can edit a few other non-PG files. I'll give that a go. And then writing/editing a theme really would be an equivalent amount of work to writing/editing a PG header file. A thought: consider a new WW-using instructor with little to no prior PG experience. And this person wants to customize the hardcopy headers/footers. Which will be a steeper obstacle for them?
I feel that option 2 is easier for people unfamiliar with PG. Also as I wrote before, I'm more than happy to change the default headers and footers to a format you are already using. I don't have the impression anyone is tied to what is there now. But I don't know what your format is yet. |
|
Prior to all of your recent hardcopy changes all an instructor needed to do was edit the default set header file and change a few lines. That was even easier than any of the options you list. They don't need to learn about |
|
In addition, to create a theme you need to know LaTeX. Most instructors know nothing about LaTeX. This makes it vastly more difficult than before to do what you are saying. |
|
You are not understanding me, and/or I am not writing things well. I have
used "create" and "edit" interchangeably and probably imprecisely. Stand by
and give me several days to meet your needs, since it takes me more time to
contribute code.
…On Fri, Jul 7, 2023, 5:50 PM Glenn Rice ***@***.***> wrote:
In addition, to create a theme you need to know LaTeX. Most instructors
know nothing about LaTeX. This makes it vastly more difficult than before
to do what you are saying.
—
Reply to this email directly, view it on GitHub
<https://protect2.fireeye.com/v1/url?k=31323334-501cfaeb-3132feb7-454455535732-66164a227ffa2ac9&q=1&e=52df48ea-addf-49d4-8764-749c9bf96832&u=https%3A%2F%2Fgithub.com%2Fopenwebwork%2Fwebwork2%2Fpull%2F2099%23issuecomment-1626397607>,
or unsubscribe
<https://protect2.fireeye.com/v1/url?k=31323334-501cfaeb-3132feb7-454455535732-b1ba6c169f543959&q=1&e=52df48ea-addf-49d4-8764-749c9bf96832&u=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FABEDOAA2HDJTIEH5ARJOMSDXPCVFFANCNFSM6AAAAAAZ3BVKCI>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
I am sorry. I don't mean to press you on this, and I don't mean to sound so harsh. I have not focused on the positive aspects of this pull requests. You have set up a versatile hardcopy theming system that looks very nice. For my purposes, I can set up my own themes that do what I want. I just think that there are many instructors that will feel the same way about the default page headers as I do. Generally, I don't want much information in those headers, and use a more condensed format. Also, since I don't use sections and recitations the default header becomes rather unbalanced. There are three lines on the left, and only one ("student name (login id)") on the right. I also don't like the links so much. For this it might help if In any case, an instructor that wants a simple way to change the header information with this system will find it much more challenging than before. Intuitively speaking the set header pg file is where one would expect to set the header for the set in the pdf. Until the recent changes was where that was done. You mention divorcing the PG set header file from PDF styling, but the header in the PDF result is more than just a style and is really a set header (it is shown for each set and user). It is part of the content. |
|
It's no problem, your feedback and reviews are very much appreciated. And I
think it is all converging to a better thing all around. I will wait to get
into details because I don't always describe things well in summary, but I
am part way through making it just as easy to edit a theme. One click, then
edit a few lines as you would with a PG header, save, and off you go.
On the last notes, note that setting the content of the PDF's
headers/footers uses different tex macros when it's the exam class, the
article class with fancyhdr package, or something else. So if you override
these things in a PG header file, you won't have flexibility to change to a
different theme (well, maybe if you use a bunch of tex conditional macros)
. But you will still have that option if you really want to when I'm done
with revisions that are underway.
…On Sat, Jul 8, 2023 at 4:27 AM Glenn Rice ***@***.***> wrote:
I am sorry. I don't mean to press you on this, and I don't mean to sound
so harsh. I have not focused on the positive aspects of this pull requests.
You have set up a versatile hardcopy theming system that looks very nice.
For my purposes, I can set up my own themes that do what I want.
I just think that there are many instructors that will feel the same way
about the default page headers as I do. Generally, I don't want much
information in those headers, and use a more condensed format. Also, since
I don't use sections and recitations the default header becomes rather
unbalanced. There are three lines on the left, and only one ("student name
(login id)") on the right.
I also don't like the links so much. For this it might help if hypersetup
were used to override the default hyperref settings that I think are
rather ugly. I always change those to make links look more like they do in
the browser when I use the hyperref package.
In any case, an instructor that wants a simple way to change the header
information with this system will find it much more challenging than
before. Intuitively speaking the set header pg file is where one would
expect to set the header for the set in the pdf. Until the recent changes
was where that was done. You mention divorcing the PG set header file from
PDF styling, but the header in the PDF result is more than just a style and
is really a set header (it is shown for each set and user). It is part of
the content.
—
Reply to this email directly, view it on GitHub
<https://protect2.fireeye.com/v1/url?k=31323334-501cfaeb-3132feb7-454455535732-b46a4e6c5840fc5f&q=1&e=37ceecc5-c3e8-4aad-89e8-ad0f74675c03&u=https%3A%2F%2Fgithub.com%2Fopenwebwork%2Fwebwork2%2Fpull%2F2099%23issuecomment-1627145251>,
or unsubscribe
<https://protect2.fireeye.com/v1/url?k=31323334-501cfaeb-3132feb7-454455535732-72405446c48c338d&q=1&e=37ceecc5-c3e8-4aad-89e8-ad0f74675c03&u=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FABEDOADZNET33S46QOM5AYLXPE725ANCNFSM6AAAAAAZ3BVKCI>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
--
Alex Jordan
Mathematics Instructor
Portland Community College
|
|
I have this close to ready to push and ask for more review, but there's something I want to check in about. (I'll be doing day job stuff through tomorrow and I can't make progress, but I hope to have it pushed by the end of tomorrow.) There are now two snippets for set header, one before the PG header and one after, which will make it possible to still override things using the PG header. I've simplified things so that hardcopy them files can only be at You can edit an existing theme file with the editor, and it saves to My question now is if I've overlooked something and made a mistake by adding this form submission button to that form. Maybe it's hard to say without looking at the code in the updated PR. But does anything occur to you as making this a bad idea? |
|
Something sounds a bit off with that structure. I can't say for certain though without seeing exactly what you have. I was thinking about the issue with overriding the pdf page headers from the pg set header. It seems that if there were TeX commands that were defined that provide the headers, say |
|
Yeah, I'm not sure about it either. The thing is, if you are going to edit a theme, you need to be at a place where you can select the theme you are about to edit. There are three existing places where that happens: (a) course config page (b) regular hardcopy production page (c) PG Editor hardcopy tab. Or else create something new. Option (c) is nice in that you are already in the editor with a form that is not already using its submit button for something else. |
assets/hardcopyThemes/basic.xml
Outdated
| \ifthenelse{\equal{\webworkPrettySetId}{}}{}{\fancyhead[L]{\ifthenelse{\thepage=1}{\large\scshape\webworkLocalizeSet: \webworkPrettySetId}{}}} | ||
| \ifthenelse{\equal{\webworkUserId}{}}{}{\fancyhead[R]{\ifthenelse{\thepage=1}{\bfseries\webworkLocalizeUsername: \webworkUserId}{}}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To follow up on #2099 (comment), if this were
| \ifthenelse{\equal{\webworkPrettySetId}{}}{}{\fancyhead[L]{\ifthenelse{\thepage=1}{\large\scshape\webworkLocalizeSet: \webworkPrettySetId}{}}} | |
| \ifthenelse{\equal{\webworkUserId}{}}{}{\fancyhead[R]{\ifthenelse{\thepage=1}{\bfseries\webworkLocalizeUsername: \webworkUserId}{}}} | |
| \fancyhead[L]{\webworkAssignmentCourseInfo} | |
| \fancyhead[R]{\webworkUserInfo} |
and all of the themes consistently used \webworkAssignmentCourseInfo and webworkUserInfo like this, then in the hardcopy set header file one could do
\renewcommand{\webworkAssignmentCourseInfo}{My custom left header content}
\renewcommand{\webworkUserInfo}{My custom right header content}
and reliably modify the headers for all themes without concern of if the exam class or fancyhdr package were used by the theme.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note the theme itself could also use \renewcommand to change the default headers for the theme. The basic.xml theme could do this to get the different headers you currently have set for this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll do something like this and update the branch. The purpose of the \ifthenelse is, in part, so that if you use the theme from the Editor (where there is no set ID or user ID), it gives no header content at all. But maybe it would be better to still show whatever the skeleton of the header would be. It's also messier with the ones relying on fanccyhdr where I don't think there is anything special to single out doing the first page headers differently than running headers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So the theme could do
\renewcommand{\webworkAssignmentCourseInfo}{%
\ifthenelse{\equal{\webworkPrettySetId}{}}{}%
{\ifthenelse{\thepage=1}{\large\scshape\webworkLocalizeSet: \webworkPrettySetId}{}}%
}
\renewcommand{\webworkUserInfo}{%
\ifthenelse{\equal{\webworkUserId}{}}{}%
{\ifthenelse{\thepage=1}{\bfseries\webworkLocalizeUsername: \webworkUserId}{}}%
}
\fancyhead[L]{\webworkAssignmentCourseInfo}
\fancyhead[R]{\webworkUserInfo}
That should do the same thing in any case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I will change \webworkAssignmentCourseInfo and \webworkAssignmentCourseInfo to have generic names like \webworkLeftHeader and \webworkRightHeader and add commentary in the right places about customizing them. Probably add \webworkCenterHeader too. And as I've noted, I'll change the default content of these to make it less possible for a left and right header overstrike.
Fix some issues that have cropped up if a user attempts to access the editor without sufficient permissions.
| my $setID = $c->stash('setID'); | ||
| my $problemID = $c->stash('problemID'); | ||
|
|
||
| return 'Editor' unless $c->{file_type}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just realized this should be return $c->maketext('Editor') unless $c->{file_type};. My bad. Can you make that change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I'll update it.
|
I changed the default There's an issue with using the footer to insert a copyright claim. It can happen that the footer is redefined to be a copyright claim, but then the set extends to one more page because of some unresolved content at that point in the processing. I tried many things to delay the resetting of the footer content, but it was always either not enough of a delay, or delayed it so much that the last page ended up without the copyright claim. So as things stand, occasionally the last two pages get a copyright claim. I can live with this (after wasting too much time battling it). But if anyone would like to try some other approach, you might have more success than I did. Aside from a footer, there are other ways to claim copyright. It could just be written as the last piece of content like it was before. Or it could be done with some kind of watermark or with thee |
86f3a49 to
bfd2960
Compare
bfd2960 to
89f50d0
Compare
This comes with the risk of the copyright claim moving to a new page though. |
|
I think the new LaTeX commands for the headers and footers make this quite versatile. Thanks for making that change. I was unable to reproduce the footer issue, but that probably takes just the right (or rather wrong) problem lengths. |
|
Thanks for the suggestions. The sty and theme files looks a lot cleaner now. Yeah, I happen to be testing with a set where the last problem is just a bit too long and gets pushed to a new page with all of the two-column themes. |
|
Oh wait. I found a set that gave the issue with the copyright. That is not to serious. |
|
I can think of one way to handle the copyright issue (that is too much work that I do not plan to do). That would be to change how hardcopies are made, so that each set is its own PDF. Then merge the PDFs at the end. There are several problems with this though, given how we do things now. But it would help address this issue because there are several tools for dealing the actual last page of a PDF, and those tools could make sure it only lands on the last page of each set. |
|
For now, this should work. I think that trying to rework hardcopy generation to generate separate PDF's and merge them is a bit ambitious at this point. I plan to release 2.18 next Thursday. |
|
It's not only ambitious, I think I would not want the side effect of effectively forcing a newpage between sets. |
OK, I'll clean up my |
|
Looking at this with fresh eyes. Noticed that if I have a long problem (extends the the length of the page), the boxed-rows themes, cuts off the end of the problem. I tried to add 'breakable' to the |
|
There is something that I just noticed that needs to be changed. Previously the |
|
Yeah, |
|
I'll add |
pstaabp
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is ready to go after multicols is added in.
|
I made the |
This pairs with openwebwork/pg#870.
This refactors how hardcopy themes work. Things to note:
examclass.assets/hardcopyThemes.READMEinassets/hardcopyThemes, or just copy one of the existing theme files.snippets. Those are gone. All ofsnippetsis gone. Thepgfiles that were there are now inassets/pg.XML::LibXML. I had trouble installing it with cpanm, but no trouble usingyum(sudo yum install perl-XML-LibXML).localOverrides.confneed updating with this PR.I expect there will be things to tweak about the themes. But I left the
\vfillin place. I think th issue I am seeing there with bad page breaks on long problems is not new to 2.18. But this PR will make it easy to adjust an existing theme or add a new one.