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

Fillin overhaul #1727

Closed
wants to merge 1 commit into from
Closed

Fillin overhaul #1727

wants to merge 1 commit into from

Conversation

Alex-Jordan
Copy link
Contributor

This removes latex.fillin.style (so there will need to be an -announce message) just for that.

It puts publication/common/fillin/@textstyle and @mathstyle in place in publication files. Each may be underline (default for text), box, or shade (default for math).

With shade becoming default for math (an improvement, I think), that is another reason for an -announce message.

The text fillin can compress in LaTeX up to 20% to help with line fitting. The text fillin in HTML puts classes in place, but a separate CSS PR is needed to actually make things happen there.

The math fillin templates are all in -common. In the end I found a way to make that work either way.

Text fillin uses @characters with default 10. Math fillin can use @characters or like @fill="\frac{x+1}{2}" with default @fill="XXX". fill trumps characters.

This does not improve accessibility for fillins. It's already OK for fillins in text. And things are not good for fillins in math and it stays that way with these commits. But I have a lead on @pkra's aria-label MathJax extension that might work out separately, later. That extension is not playing will with \phantom right now.

Lots of doc cutting and editing for this. I tried to pay lots of attention, but a second set of eyes on that would be good.

Not sure how you want to handle schema here. Now two types of fillin, one in math and one not?

@Alex-Jordan
Copy link
Contributor Author

This replaces #1710 .

@rbeezer
Copy link
Collaborator

rbeezer commented May 30, 2022

A lot of work here! ;-)

Comments based on visual review. Feel free to pile on commits, or force-push, neither is a problem.

  • DOCUMENT-FILTER is a nice thing, but I don't see it being used anywhere?
  • I'm on a campaign to move out as many warnings and errors as possible when the schema (or validation-plus) can determine the problem. For example, to address your shema question, I expect to add a FillinText and FillinMath pattern, and these will keep the roles of @characters and @fill straight. There might even be something like an xs:number type that will keep @characters under control. Can you review messages with an eye to not duplicating the role of schema validation?
  • Publisher variables should do its best to respect options selected with old string parameter. In other words, we should do our best to not abandon selections that we have replacements for. Should be many examples, test for this situation first.
  • When you respect the string parameter, you may need/want to unwind the otherwise/if construction on the end of the two publisher variables. I think I like a final otherwise that guarantees the default value, even if it may not logically be necessary.

@Alex-Jordan
Copy link
Contributor Author

DOCUMENT-FILTER

Oh! I had that for something that was cut once I found a way to do shade in math that worked for both LaTeX and MathJax. Can cut with the next force push.

The new publisher variables here apply to all output forms. But the old stringparam latex.fillin.style is only for LaTeX. So I don't see a way to make the new publisher variables respect the old stringparam.

The only way I see to continue to respect the old stringparam is to do things with it inside the LaTeX stylesheet. I will do that if you'd like. The change here would affect anyone who uses latex.fillin.style="box". Maybe just my opinion, but moving to the text default of underline and math default of shade feels like improvements in both places. I was thnking it's like an opportunity to really kill an old stringparam without anyone complaining.

In the next force push I will modularize the latex macro definition so that the beamer template just calls it instead of repeating that code.

@rbeezer
Copy link
Collaborator

rbeezer commented May 30, 2022

The new publisher variables here apply to all output forms.

OK, I see. Make that distinction clear then for -announce. "LaTeX output will revert to the default value which is 'underline'. An election for "box" needs to use the new publisher variable." Or something along those lines, I'm not checking that what I just said is correct.

And maybe review the deprecation message. We want to reduce panic. ;-)

@Alex-Jordan
Copy link
Contributor Author

Will the schema be able to say that a fillin inside math can have @characters, or it can have @fill, but it cannot have both?

@rbeezer
Copy link
Collaborator

rbeezer commented May 30, 2022

Yes. Easy-peasy. (I think.)

@Alex-Jordan
Copy link
Contributor Author

OK, Take 2 is ready.

Even if everything here looks good, still need:

  • schema updates
  • CSS for .fillin.underline, .fillin.box, .fillin.shade
  • message to -announce

Should I try the schema updates or is it trivial for you?

@rbeezer
Copy link
Collaborator

rbeezer commented May 31, 2022 via email

@Alex-Jordan
Copy link
Contributor Author

Got it.

@davidfarmer At Subsection 4.7: Some Paragraph-Level Markup
https://spot.pcc.edu/~ajordan/temp/interesting-corollary.html#subsection-paragraph-markup
I repeated a paragraph with fillins three times.

The first paragraph has .fillin.underline. Then .fillin.box. Then .fillin.shade.

We need plain .fillin to continue looking like it looks now. And .fillin.underline should look the same way.

.fillin.box should put a border around the span and give it an appropriate height.

.fillin.shade should give it an appropriate height, but no border. And make the background color be gray. Specifically #e6e6e6.

I would add these except I know CSS is in so much transition right now.

@davidfarmer
Copy link
Contributor

davidfarmer commented May 31, 2022 via email

@Alex-Jordan
Copy link
Contributor Author

My most recent message here has an example posted. A "real" example will only have one of the fillin classes so I doctored that one to have all three.

rbeezer added a commit that referenced this pull request Jun 2, 2022
@rbeezer
Copy link
Collaborator

rbeezer commented Jun 2, 2022

Very good! That's all a big improvement. I'll rebuild website shortly.

Removed a mention in the sample article of @characters being deprecated. And exploded out into severtal topical commits keeping all references to the work being yours.

@rbeezer rbeezer closed this Jun 2, 2022
@rbeezer
Copy link
Collaborator

rbeezer commented Jun 2, 2022

Closed this too soon. But schema work is now done at: b65452b

I classified a fill-in blank for text as a "generator" rather than a "character". May have lost a few odd-ball uses that didn't really make any sense anyway. And a new pattern for the math context. Tests well against the sample article.

@Alex-Jordan
Copy link
Contributor Author

Thanks! Is it ready for an announce message? Maybe after @davidfarmer styles the HTML blanks?

@rbeezer
Copy link
Collaborator

rbeezer commented Jun 2, 2022

Knew I was forgetting something. Yes, fire away proudly on -announce. I thought shade looked quite good already in HTML. Your call on CSS timing.

@Alex-Jordan
Copy link
Contributor Author

Math fillin in HTML is covered. CSS is needed for HTML text fillin. But the default is underline and that is already what you get even without new CSS. So I'll do the -announce now. Now rush on box and shade CSS for text fillin.

@Alex-Jordan
Copy link
Contributor Author

Alex-Jordan commented Oct 11, 2022 via email

@Alex-Jordan
Copy link
Contributor Author

I think the CSS changes for this was either never done, or reverted during the overhaul. It has gone unnoticed this long because the default is OK. However, in ORCCA I think I will make text fillins have the shade style, and I noticed it was not working.

If you go here: Paragraph
https://pretextbook.org/examples/sample-article/html/interesting-corollary.html#p-238

There is a span.fillin.underline. It looks fine. Use the inspector to change it to span.fillin.box or span.fillin.shade, and nothing changes. I'm comfortable going in to make these CSS additions, but I want to check in with @davidfarmer first.

@davidfarmer
Copy link
Contributor

davidfarmer commented Mar 29, 2023 via email

@Alex-Jordan
Copy link
Contributor Author

Let me know if something does not look right when those are actually
in the sample article.

It is a publisher switch to use one of the options globally, so it will never be the case that you can see them all on the same page with the untouched sample article. But using the inspector to change a class should be OK to get a glimpse of the other two styles.

@rbeezer had some suggestions at last week's drop-in about where the baseline of these things should be. Rob, if you look here, what would you change? I can't recall the differences (if any) for the baseline between underline, box, and shade.

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

3 participants