PreTeXtBook / pretext

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

Youtube playlist #947

Closed Alex-Jordan closed 5 years ago

Alex-Jordan commented 5 years ago

Starting with this to see if anything about the documentation raises flags for you. I'll continue to commit parts to this branch and you could package it into one commit when it's all done.

There is some magic distinction between YT users and channels that I cannot fully grok. Sometimes I can get a channel embedded, sometimes not. Sometimes a user's uploads, sometimes not. So I am bailing on that kind of extension for now. Playlists only.

rbeezer commented 5 years ago

Looks good. Let's put "playlist" into <term> like it is being defined. Generally, I'm trying not to make the Author's Guide a compendium of one example of everything, but just enough to whet one's appetite, and enough guidance for someone to figure it out. So feel free to make one of each thing for the sample article, and you only need to provide descriptions of options in the Author's Guide, though there should be at least one playlist example there. Maybe a small table of the different attributes with an example specifications and an explanation? Single, list, playlist, channel?

Alex-Jordan commented 5 years ago

XSLT in place for HTML. Do you want to take a look and comment?

Alex-Jordan commented 5 years ago

Figures 16.12 and 16.13 here, if you'd like to peek at output: http://spot.pcc.edu/~ajordan/temp/section-video.html

rbeezer commented 5 years ago

Looks good. Didn't require too much surgery, no?

Right now, if an author uses a single-video ID and includes a stray space before or after, that one video will get packaged as a 1-video playlist, no? How about a plain normalize-space() before even entering the choose? Then your test for the multiple-ID scenario will be more fool-proof? (And it might mean you don't need the later instance of normalize-space(), which I have not checked carefully.)

Alex-Jordan commented 5 years ago

I can do what you are suggesting. It feels

Alex-Jordan commented 5 years ago

Oops

Alex-Jordan commented 5 years ago

I was going to say, it feels off balance in one way to correct the author's stray space, but not a stray comma if they left one. But it sounds easy to do.

rbeezer commented 5 years ago

Just reading commits (not the cumulative effect, locally), but I think I have a handle on current state.

Looks fine. But see if you like this better. Basically get commas out of the way early and add back last.

Sorry for the delay, startups have been keeping me busy.

Alex-Jordan commented 5 years ago

OK, I was avoiding introducing a variable $youtube. But happy to do that.

The stylesheet could be cleaner if the URL were allowed to be built with an ampersand on the first option. Like https://www.youtube.com/embed?&amp;listType=playlist instead of https://www.youtube.com/embed?listType=playlist. The ?&amp; appeals to me because on those rare occasions when it matters, it is easier to cut and paste the options out of a URL. The only down side is that the simpler ? looks more "human-written". But both function. Do you have an opinion on this?

Alex-Jordan commented 5 years ago

LaTeX YT playlist is now here for your first impressions. I haven't actually tested so maybe there is a typo or something. Moving on to the script to get it to scrape first image from a list-defined playlist. Will do a complete test after that.

Alex-Jordan commented 5 years ago

As happens, I immediately see issue. Please disregard until I signal the whole thing is tested and ready for review.

rbeezer commented 5 years ago

No firm opinion.

But am wondering, when will the reader see this URL and be chopping out options? In the raw HTML? Not checking anything carefully, just presuming the reader sees an embedded video within some page in their browser and so does not have easy access to the URL?

On 09/05/2018 02:07 PM, Alex Jordan wrote:

OK, I was avoiding introducing a variable |$youtube|. But happy to do that.

The stylesheet could be cleaner if the URL were allowed to be built with an ampersand on the first option. Like |https://www.youtube.com/embed?&amp;listType=playlist| instead of |https://www.youtube.com/embed?listType=playlist|. The |?&| appeals to me because on those rare occasions when it matters, it is easier to cut and paste the options out of a URL. The only down side is that the simpler |?| looks more "human-written". But both function. Do you have an opinion on this?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/rbeezer/mathbook/pull/947#issuecomment-418881690, or mute the thread https://github.com/notifications/unsubscribe-auth/ABy2cj49Z652j7rSik5ntVK7D2qD-YILks5uYDz3gaJpZM4WM_Ta.

Alex-Jordan commented 5 years ago

A reader doesn't. An instructor may pop into the browser to grab the url to duplicate in some LMS.

I do this kind of thing to change seeds in ww problems for testing, so it's on my mind. Ignoring this, it's a question of cleaner XSLT versus "prettier" output.

On Wed, Sep 5, 2018 at 3:40 PM, Rob Beezer notifications@github.com wrote:

No firm opinion.

But am wondering, when will the reader see this URL and be chopping out options? In the raw HTML? Not checking anything carefully, just presuming the reader sees an embedded video within some page in their browser and so does not have easy access to the URL?

On 09/05/2018 02:07 PM, Alex Jordan wrote:

OK, I was avoiding introducing a variable |$youtube|. But happy to do that.

The stylesheet could be cleaner if the URL were allowed to be built with an ampersand on the first option. Like |https://www.youtube.com/embed?&amp;listType=playlist| instead of |https://www.youtube.com/embed?listType=playlist|. The |?&| appeals to me because on those rare occasions when it matters, it is easier to cut and paste the options out of a URL. The only down side is that the simpler |?| looks more "human-written". But both function. Do you have an opinion on this?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/rbeezer/mathbook/pull/947#issuecomment-418881690, or mute the thread https://github.com/notifications/unsubscribe-auth/ ABy2cj49Z652j7rSik5ntVK7D2qD-YILks5uYDz3gaJpZM4WM_Ta.

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/rbeezer/mathbook/pull/947#issuecomment-418904398, or mute the thread https://github.com/notifications/unsubscribe-auth/AEg3ABfOa_Wtxa6XL46H0xx_EFYpddbNks5uYFLLgaJpZM4WM_Ta .

-- Alex Jordan Mathematics Instructor Portland Community College

Alex-Jordan commented 5 years ago

OK, tested and ready for review. Perhaps in this order:

  1. Check out the sample-article HTML output here: http://spot.pcc.edu/~ajordan/temp/section-video.html Figures 16.12 and 16.13.

  2. Check out the PDF output here: http://spot.pcc.edu/~ajordan/temp/derivatives.pdf Page 67. You will see the issue with the "static-caption" running off its bounded area. This is an issue that is orthogonal to putting in the playlists, but I think the sample-article did not have an example that revealed it. Should it be addressed in this PR? And if so, what would you like to see? URL's that wrap? Just print the IDs?

  3. Examine the PR as a whole. Looking at commits is not so helpful now that there have been multiple commits to each file touched.

  4. Ultimately this will make a thumbnail for the "enumerated" playlist. Should I commit that to the repo like all other youtube thumbnail images for the sample article?

rbeezer commented 5 years ago

On 09/05/2018 04:21 PM, Alex Jordan wrote:

4.

Ultimately this will make a thumbnail for the "enumerated" playlist. Should
I commit that to the repo like all other youtube thumbnail images for the
sample article?

Yes, please include the thumbnails. We want newcomers to build the sample article without running the "mbx" script.

More later.

Alex-Jordan commented 5 years ago

Thumbnail committed. And a bunch of cleanup I somehow missed on my last pass.

rbeezer commented 5 years ago

You will see the issue with the "static-caption" running off its bounded area. This is an issue that is orthogonal to putting in the playlists, but I think the sample-article did not have an example that revealed it. Should it be addressed in this PR? And if so, what would you like to see? URL's that wrap? Just print the IDs?

I need to do more work on static versions of interactive portions, so I'll think more about captions then.

rbeezer commented 5 years ago

Nice work, this is a great enhancement. Corrected one small typo. Otherwise, repackaged as six topical commits, each with your name on it.

Feel free to run your own advertisement on pretext-announce if you think it is time.

Thanks! What's next?