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

New dev server #219

Closed
wants to merge 13 commits into from
Closed

New dev server #219

wants to merge 13 commits into from

Conversation

kristojorg
Copy link
Contributor

@kristojorg kristojorg commented Jul 2, 2021

This PR adds a server.ts file which is our new dev server. It uses r2-streamer-js to set up the express server, but also statically serves the contents of /viewer and /dist so that we don't have to manually copy files around after building.

Clicking on a publication will give you the option to view it using any of the examples in /viewer.

We need to wait for readium/r2-streamer-js#56 to merge and a new release before we can merge this though.

@aferditamuriqi what do you think about moving /viewer into /examples/html? That was the /examples folder holds all the code for the examples, whether react or html based.

This also checks in a single example epub, but we can remove it and gitignore it if you'd rather do that. The folder for sample epubs is now: /examples/epubs.

To give this a try, run npm run build then npm run examples on this branch.

@kristojorg kristojorg changed the base branch from production to develop July 2, 2021 12:47
@kristojorg kristojorg changed the base branch from develop to v2 July 2, 2021 12:47
@kristojorg kristojorg added the V2 label Jul 2, 2021
@kristojorg kristojorg requested a review from aferditamuriqi July 5, 2021 11:24
@kristojorg kristojorg marked this pull request as ready for review July 5, 2021 11:24
@kristojorg kristojorg marked this pull request as draft July 5, 2021 11:25
@kristojorg
Copy link
Contributor Author

There hasn't been any comments on readium/r2-streamer-js#56 so maybe we merge it into our fork of r2-streamer-js and then publish our own version of it? Or we could ask them to merge this

@danielweck
Copy link

Hello, I have been thinking about it. I will get back to you shortly with comments.

@kristojorg
Copy link
Contributor Author

@danielweck thanks so much!

@aferditamuriqi
Copy link
Member

aferditamuriqi commented Jul 5, 2021

what do you think about moving /viewer into /examples/html? That was the /examples folder holds all the code for the examples, whether react or html based.

@kristojorg That would be fine, we just need to make sure that any of the injectable examples are still working as well. but from what i understand we will use dist then directly, for the injectables, same as the reader.js and reader.css ?

@kristojorg
Copy link
Contributor Author

kristojorg commented Jul 5, 2021

Yep exactly we would be serving dist at /viewer so any page in /viewer can request anything within dist:

viewer/index.html

<script src="reader.js" />
...
<link rel="stylesheet" href="reader.css" />

@kristojorg
Copy link
Contributor Author

How should I make sure that the injectables examples still work? How do I run them / which ones are they?

@aferditamuriqi
Copy link
Member

For example here https://github.com/d-i-t-a/R2D2BC/blob/production/viewer/index_dita.html#L554
i could think the injectables could be compiled and placed as well into examples /examples/injectables and used statically from there. i wouldn't use them from dist, as these injectables are actually examples. and anyone can create their own outside of r2d2bc and inject.

@aferditamuriqi
Copy link
Member

aferditamuriqi commented Jul 5, 2021

the example injectables are specified here https://github.com/d-i-t-a/R2D2BC/blob/feature/new-streamer/webpack.config.js#L54

You see there that glossary and footnotes are really just examples, the style.css is not specified in the webpack for example.

@kristojorg
Copy link
Contributor Author

Ok right now injectables are compiled into dist/injectables, which is what happened with them before. I think that is the safest, but we could compile them directly into examples/injectables if you would rather that (I think this'll work)

@kristojorg kristojorg marked this pull request as ready for review July 26, 2021 13:15
@kristojorg
Copy link
Contributor Author

Closing in favor of #243

@kristojorg kristojorg closed this Aug 9, 2021
@kristojorg kristojorg deleted the feature/new-streamer branch August 9, 2021 07:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

3 participants