internetarchive / bookreader

The Internet Archive BookReader
https://openlibrary.org/dev/docs/bookreader
GNU Affero General Public License v3.0
996 stars 418 forks source link

Consolidate most "BookReaderJSSimple" demo files #354

Open cdrini opened 4 years ago

cdrini commented 4 years ago

Most of the demo files differ only in:

  1. plugins used
  2. BookReader options
  3. Book data

All the demos that use BookReaderJSSimple have the same data. Consolidate those to be the same HTML file, with URL parameters for specifying any extra BookReader options.

Summary of Requirements

jimchamp commented 4 years ago

I'll take this up.

jimchamp commented 4 years ago

Do we want the query string to be human-readable? Something like &extraOpts={"startFullscreen": true} will need to be encoded in order for the URL to be valid.

cdrini commented 4 years ago

Awesome! In the spirit of YAGNI, I think it's ok if it's not human readable. If that ends up being too annoying, we can add it later :)

jimchamp commented 4 years ago

@cdrini Is is alright if I open a draft PR for this tomorrow? I want to make sure my approach is acceptable before consolidating all of the files.

cdrini commented 4 years ago

Absolutely, please do!

jimchamp commented 4 years ago

This is a bit more involved than I originally thought. I'm trying to import the plugins programmatically, but if they are not loaded in the correct order the demo doesn't work properly.

I am able to pass the extra options successfully via a query string, though.

cdrini commented 4 years ago

Hmm, can you push your code up? If they're loaded in the same order as in the URL, does that not work?

jimchamp commented 4 years ago

Just pushed it up. FYI: It looks like there are 10 total demo files that use BookReaderJSSimple. Of those, only 3 use plugins.

jimchamp commented 4 years ago

I've marked this ready for review. The only demo that I didn't include in the refactor is the preview pages demo. I suspect that the length of the query string for that demo will exceed the URL length limits...