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

Missing package-level rendition_flow, rendition_orientation, rendition_spread metadata (JSON to shared-js) #42

Closed
danielweck opened this issue Dec 2, 2014 · 5 comments
Assignees

Comments

@danielweck
Copy link
Member

Issue first mentioned in #39
(see comment thread)

@danielweck
Copy link
Member Author

Ah, here's the culprit:
https://github.com/readium/readium-sdk/blob/develop/Platform/Android/src/org/readium/sdk/android/Package.java#L423
As you can see, only rendition_layout is implemented!
Missing: rendition_flow, rendition_orientation, rendition_spread.
@jccr would you mind looking into this JNI code if you have spare cycles? Thank you very much!

@danielweck
Copy link
Member Author

PS: instead of just fetching the value in the toJSON() function, could we store the values locally as it's already done with page-progression-direction (for example):
https://github.com/readium/readium-sdk/blob/develop/Platform/Android/src/org/readium/sdk/android/Package.java#L138

@danielweck danielweck changed the title authored rendition:spread not respected, suspected bad JSON info passed to shared-js? Missing package-level rendition_flow, rendition_orientation, rendition_spread metadata (JSON to shared-js) Dec 3, 2014
@danielweck
Copy link
Member Author

Note: this is really a follow-up to readium/readium-sdk#128 (where the missing spine-level metadata was added). This separate issue is for the package-level metadata.
PS: I have re-assigned this issue to myself.

@danielweck
Copy link
Member Author

Addressed in feature branch / Pull Request:
readium/readium-sdk#134
#43

@danielweck
Copy link
Member Author

@jccr I implemented support for package-level rendition metadata, which required fixing the spine-level accessor methods (to cascade up into the package, when properties are not explicitly set by spine items). Could you please take a look at the Pull Request (linked above). You'll see that I decided to pass the Package instance directly into the appropriate methods, rather than to store a permanent reference to the Package object at SpineItem construction time. The latter would require modifying the JNI construction code (marshalling of the Package JNI pointer), which felt unnecessarily complicated. However, if you think it is worth the effort, let's refactor this before merging to develop. Thanks :)

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

No branches or pull requests

2 participants