Skip to content

Add reached-start and reached-end CustomEvents #65

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

DialBird
Copy link

@DialBird DialBird commented May 14, 2025

I implemented a CustomEvent that is fired when the first or last page is reached and the user tries to go further.

@DialBird DialBird changed the title Add atStart and atEnd to FixedLayout Add reached-end CustomEvent May 14, 2025
@DialBird DialBird force-pushed the feature/add_at_end_and_start_to_fixed branch from 6f0bbaf to 58775bc Compare May 14, 2025 02:14
@DialBird DialBird changed the title Add reached-end CustomEvent Add reached-start and reached-end CustomEvents May 14, 2025
@johnfactotum
Copy link
Owner

Is there a need for the events if you can check it in the relocate event?

@DialBird
Copy link
Author

DialBird commented May 18, 2025

Hi @johnfactotum

The two events implemented this time are of a type that cannot be obtained with the relocate event.

The reason is that the page is not redrawn when you try to go to the next page after reaching the last page (or before the first page).

The event that says that the page was about to be turned but was interrupted because it was the last (first) page and could not go any further had to be written directly in the logic of fixed-layout.js and paginate.js.

@johnfactotum
Copy link
Owner

What's the use case for that? And for the fixed-layout renderer at least, since prev() and next() aren't called anywhere inside the module, one should handle it where those are called. Or, rather than an event, the methods could return something (or throw) that indicates whether the page turn is successful or not.

@DialBird
Copy link
Author

DialBird commented May 21, 2025

This is how I’m thinking about the use of the reached-start and reached-end events.

Use Case

The main reason is to support common features in e-book readers—for example, showing a “Please leave a review” message when a user finishes a book, or triggering actions when the user reaches the start or end. These are common patterns in many e-reading apps, and using events is a simple and clean way to handle them.

Why Dispatch Events Inside the Renderer

I understand your point about handling this at the place where prev() and next() are called. But I chose to handle it inside the renderers because fixed-layout.js and paginator.js work in very different ways and manage their state differently.

If we moved the boundary-checking logic outside (e.g., to view.js), we would need to expose the internal state of each renderer. This would make the code more tightly coupled and harder to manage. Since each renderer knows its own state best, it makes sense for them to emit events when they reach the start or end.

Why Use Events Instead of Return Values or Errors

I did consider other options:

  • Throwing errors would require adding try-catch blocks around every call to prev() and next(), which would make the code harder to read and maintain.
  • Returning booleans would change the method signatures and could break existing code that uses these methods.

Using events is the most flexible and least disruptive approach. Components that care about these events can listen to them, and others can just ignore them. This keeps the current API unchanged while adding new functionality.

Also, the codebase already uses events to communicate state changes, so this follows the existing design pattern.

Would you be open to keeping this implementation, or is there anything else I should consider?

@johnfactotum
Copy link
Owner

That sounds to me that you'd want to handle it in the application. It's basically the same as, e.g. disabling prev/next buttons at the start/end. So for example, rather than disabling the next button, you'd make it trigger the message if atEnd is true.

You can also do something more sophisticated, like detecting whether the end is reached by considering semantic information contained in the book (navigation documents, epub:type, role, etc.), to account for cases where there's backmatter that aren't typically read through.

The only problem I see is with touchscreen swiping (which currently is only supported by the paginator), which is internal to the paginator at the moment (though it might be better to move it out or make it public). There's no overscroll, and even if there is, it's unclear how it'd work.

Perhaps the cleanest way for this specific use case would be to insert an empty dummy section at the end. You can then show the message when relocating to the section.

@DialBird
Copy link
Author

Thank you for your detailed feedback. I'd like to clarify a few points:

  • If I understand correctly, your stance is that we should rely on EPUB semantic information (navigation documents, epub:type, role, etc.) as the basis for triggering these events. Is that accurate?
  • My approach actually comes from a different perspective - I'm trying to create a solution that doesn't rely on semantic information, as I want it to work reliably with any EPUB file regardless of how well it adheres to semantic standards. In my experience, many EPUB files have inconsistent or missing semantic markup, so I wanted a solution that would work universally.
  • I’d like to clarify a possible misunderstanding regarding my implementation.
    You suggested “disabling prev/next buttons at the start/end,” but in fact my code does not alter the existing navigation behavior. It merely dispatches events when a boundary is reached, without interrupting the navigation flow. The original navigation logic remains intact—I’m only providing optional notifications for consumers to listen to if they wish.
  • Regarding the touchscreen swiping issue, I may have misunderstood your concern. In my testing, I've actually confirmed that both implementations (fixed-layout and paginator) respond properly to swipe gestures. The events are triggered correctly when reaching boundaries through swiping in both renderers.

If our fundamental approaches to this problem are different, I understand that this PR might not align with the project's direction. I'm happy to reconsider my approach or close this PR if you feel it doesn't fit with your vision for the library. Please let me know your thoughts on how you'd prefer to proceed.

@johnfactotum
Copy link
Owner

If I understand correctly, your stance is that we should rely on EPUB semantic information

No, I mentioned it only as an example of what applications might do. It's probably not terribly useful in practice.

No—my point is that checking whether it's at the start/end is not really any different from checking whether the user has navigated to any other specific point in the book apart from the start/end. Namely, you would check the location provided by the relocate event.

You suggested “disabling prev/next buttons at the start/end,”

Again, this is just an example (I said "e.g."). The point is that it is something that can be done without a separate event. And your use case is quite similar to it, from which I then feel that it can similarly be implemented using just the existing relocate event.

In my testing, I've actually confirmed that both implementations (fixed-layout and paginator) respond properly to swipe gestures.

The fixed-layout renderer does not handle touchscreen events. Probably it's responding to touches from the wheel event.

At any rate, my concern was about 1:1 gestures (that is, the page will following your movement, whether it's touchscreen or touchpad), which the fixed-layout renderer does not support. As I said, while the paginator supports 1:1 touch gestures, since there's no overscroll, i.e. if it's at the start/end, swiping will not move the page in that direction at all, that means these events of yours, if they were to be provided for the paginator as well, will never be triggered when using the touchscreen.


To recap, the gist of my comment is basically: if it's possible to achieve the desired result with the relocate event, there would be no need for adding the events. So is there any reason why the relocate event is not enough?

Look at Epub.js's API, for example. atStat and atEnd are properties of the location object, which you'd typically access from the relocated event. There are no separate events for them.

@DialBird
Copy link
Author

I understand.

I felt that if I knew whether my status was atStart or atEnd, I could take the next or previous action in that state, so that's how I'll handle the event itself this time.

Thank you very much for your time in this discussion.

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