search

PreTeXtBook / pretext

PreTeXt: an authoring and publishing system for scholarly documents
https://pretextbook.org
Other
254 stars 203 forks source link

Knowl rewrite #2109

Closed ascholerChemeketa closed 3 months ago

ascholerChemeketa commented 3 months ago

Rewrite of knowl.js to remove JQuery dependency Incorporates knowl accessibility improvements discussed in this closed PR: https://github.com/PreTeXtBook/pretext/pull/2107

Opened as draft. Others won't easily be able to test this until pretext points at _static for js/css.

Tested and working:

Needed testing:

rbeezer commented 3 months ago

Thanks, @ascholerChemeketa!

Others won't easily be able to test this until pretext points at _static for js/css.

Right. That is about half-done, and I hope to gbet it out on Monday.

rbeezer commented 3 months ago

Thanks very much @ascholerChemeketa. I've had a peek.

Aside: this is an example of where allowing extra flexibility then makes it harder for us to serve some readers.

davidfarmer commented 3 months ago

There is a limit to what I can do just by looking at the code.

Can we have a live example for testing?

rbeezer commented 3 months ago

https://pretextbook.org/beta/knowl-2024-01-22

Is this still necessary? I mean the "live" part. I built this locally and it tests out just fine.

rbeezer commented 3 months ago

Built the WW sample chapter and it all seems functional. Problems inside knowls, knowls inside of problems. So I'd say if there are any gotchas, they are not pervasive.

ascholerChemeketa commented 3 months ago

Yes, testing this one there will be important.

There were similar limits to what I could do where I was just forced to read the code and had no test examples (e.g. WebWork). I fully anticipate some issues to fix.

Veering into meta-discussion about process...

Live previews should not be needed anymore for anyone who knows how to build without the CLI - just pull the PR and do a build of your favorite book (https://stackoverflow.com/a/30584951).

Building/updating them doesn't sound like a great use of Rob's time, but they would make "drive by" testing easier, especially for non-devs. They could be automated through GitHub. We could also add minimal hooks to github that called a piece of code on pretextbook.org to automatically build the sample-book for PRs into a predictable folder ( pretextbook.org/development/PR/NUMBER/).

On Mon, Jan 22, 2024 at 9:57 AM Rob Beezer @.***> wrote:

https://pretextbook.org/beta/knowl-2024-01-22

Is this still necessary? I mean the "live" part. I built this locally and it tests out just fine.

— Reply to this email directly, view it on GitHub https://github.com/PreTeXtBook/pretext/pull/2109#issuecomment-1904521857, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB3NHFLEJDXEJ5KPHXACZSLYP2R7ZAVCNFSM6AAAAABCEGGDZ2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMBUGUZDCOBVG4 . You are receiving this because you were mentioned.Message ID: @.***>

bnmnetp commented 3 months ago

@ascholerChemeketa - There is a WeBWorK sample book in the pretext core repo now.

I like the idea of saving rob the hassle by automating the build/deploy of preview/beta books. If Rob likes the idea, happy to help set up the github actions.

rbeezer commented 3 months ago

Thanks, @bnmnetp and @ascholerChemeketa! Something like thius would be great.

Veering into meta-discussion about process...

How about opening a thread on pretext-dev, you can just copy the above. (I think somebody else suggeted something similar.)

ascholerChemeketa commented 3 months ago

Started a thread on pretext-dev. Make sure to tune in @bnmnetp

bnmnetp commented 3 months ago

😄 👍🏻

ascholerChemeketa commented 3 months ago

@rbeezer I think this should address concerns about tags in aria-label. Now streaming the text from the existing title, which should be tag free and have some bonus info. Adds localization of "Reveal" or "Close".

ascholerChemeketa commented 3 months ago

Runestone in knowls almost ready... dropped this back to a draft while I finish that

ascholerChemeketa commented 3 months ago

Runestone is working now. If runestone component is in a knowl that is being loaded, the existing page is searched for a matching HTML id. If none is present, the component is rendered in knowl. If that id already exists on the page, this message is rendered in the knowl:

The interactive that belongs here is already on the page and cannot appear multiple times. Scroll to interactive.

Merging should wait on webwork-pretext shifting from id-ref born hidden knowls to summary/details based ones.

rbeezer commented 3 months ago

Excellent! Thanks for all the work (and patience) on this one. Rebuilt the commits, but did not change any code. One tab to remove. Merged, pushed, and website samples rebuilt.

rbeezer commented 3 months ago

Forgot to say - I did some tests a while ago on the content going to attributes and it seems to be OK. The -common template tries to only deal with text so perhaps that is why. Though I can't say I ever found a concrete definitive explanation.

ascholerChemeketa commented 3 months ago

@rbeezer - replying to your JQuery question(s):

There are other pretext js files that use jQuery. So it can't be fully purged. Yet. Eventually, it should be either purged or automatically grabbed by a build process - hosting a copy of the jquery.min.js file isn't awesome. But for now leaving a local copy of jquery.min.js in place means that any scripts that depend on jquery are usable offline. I'd say leave it alone until the JS/CSS bundling gets tackled.

I am pretty sure you can remove jquery.min.js and jquery.sticky.js

Agree there is no reason to have js_lib and js. Makes sense to merge the two.