Skip to content

Replace reading order indices with HREFs in the EPUB viewport #624

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

Merged
merged 1 commit into from
Jul 1, 2025

Conversation

mickael-menu
Copy link
Member

Replace reading order indices with HREFs in the EPUB viewport, to stay consistent with our other APIs which use HREFs instead of an index.

Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR replaces reading order indices with HREF values for the EPUB viewport, aligning the API with other parts of the system.

  • Replaces the reading order indices and progressions dictionary keys with HREFs.
  • Introduces a mapping for visible reading order resources to associate indices with their corresponding HREFs.
Comments suppressed due to low confidence (2)

Sources/Navigator/EPUB/EPUBNavigatorViewController.swift:635

  • [nitpick] Consider renaming the tuple variable 'i' to a more descriptive name (e.g., 'resource') to enhance code readability.
            progressions: visibleReadingOrder.reduce([:]) { progressions, i in

Sources/Navigator/EPUB/EPUBNavigatorViewController.swift:136

  • Confirm that the updated comment clearly conveys that the array contains HREFs rather than indices, to maintain consistency with the API design.
        /// Visible reading order resources.

@mickael-menu mickael-menu merged commit 54eb768 into develop Jul 1, 2025
5 checks passed
@mickael-menu mickael-menu deleted the adjust-viewport branch July 1, 2025 16:07
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.

1 participant