Skip to content

Conversation

alecdhansen
Copy link

I found it useful to have access to the epub's positions data for building scrub control components, etc.

Screenshot 2024-11-25 at 12 22 00 PM Screenshot 2024-11-25 at 12 22 48 PM

@alecdhansen alecdhansen changed the title feat(ios, android) send onPositions data feat(ios, android) send epub positions data Nov 25, 2024
val viewScope = viewLifecycleOwner.lifecycleScope

channel.send(ReaderViewModel.Event.TableOfContentsLoaded(model.publication.tableOfContents))
viewScope.launch {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alecdhansen why is the viewScope being used here?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

positions() and positionsByReadingOrder() are suspend functions, so I'm calling them inside a coroutine. By using this scope, everything cancels automatically if the View is destroyed, which should avoid leaks or unnecessary work.

Happy to hear your thoughts if something else would work better!

@jspizziri
Copy link
Contributor

@alecdhansen , thanks for the PRs!

After taking a look at them, I'm now wondering if all the metadata events should be combined into a single one likeonPublicationReady which would contain the ToC, Positions, and Metadata. Since they're all available at the same time I'm not sure if splitting them out into separate events provides much benefit.

Interested in your thoughts.

@alecdhansen
Copy link
Author

@jspizziri that makes sense to me. Your call! I could close these two PRs and open another one to combine the 3.

@jspizziri
Copy link
Contributor

jspizziri commented Nov 27, 2024

@alecdhansen That'd be great, thanks! We'll flag the ToC event as deprecated.

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

Successfully merging this pull request may close these issues.

2 participants