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

CFI related improvements #395

Merged
merged 34 commits into from
Jul 13, 2017

Conversation

jccr
Copy link
Contributor

@jccr jccr commented Jul 11, 2017

Improves general CFI functionality.
Fixes #384, #386. #353, #294

Introduces new Reader API calls:
getStartCfi - Get CFI of the first element from the base of the document
getEndCfi - Get CFI of the last element from the base of the document
getNearestCfiFromElement - Useful for getting a CFI that's as close as possible to an invisible (not rendered, zero client rects) element

Ran auto & manual tests to verify improvements with no regressions in the CFI area with focus on:

  • Reflowable view
    • Right to left
    • Vertical text
  • Scroll view doc & continuous

This PR also changes the way first/last visible CFI using text offsets are generated. Now they are based on a collapsed range when appropriate.
Example:

Previously a CFI anchor was generated with a 1 character offset:
First: /4/2[introduction]/2,/1:0,/1:1
Last: /4/2[introduction]/14,/1:223,/1:224

Now this anchor is still a single character but the start and end being the same start offset, hence a collapsed range or “caret” range
First: /4/2[introduction]/2/1:0
Last: /4/2[introduction]/14/1:224

This pull request is Finalized

Related issue(s) and/or pull request(s)

Most likely fixes or improves the state of these issues:
#388
#384
#386
#345
#353
#294
#269
#230

Juan Corona and others added 30 commits July 10, 2017 14:35
A good chunk of the clean up involves refactoring functions that deal with obtaining pagination offsets for better readability and consistency
Note:
Video elements (or other elements that are a "component") that is partially visible will be ignored.
It's not a that big of an issue right now and a better solution needs to be found.
It is useful for getting a CFI that's as close as possible to an invisible (not rendered, zero client rects) element
…here CFI

was passed instead of $element findPageIndexDeltaByRectangles
Juan Corona added 3 commits July 10, 2017 18:56
Return null instead of an array with an empty object. This is to match the behaviour before the recent refactoring
@jccr jccr changed the title Improved CFI nav logic General CFI related improvements Jul 11, 2017
@jccr jccr changed the title General CFI related improvements CFI related improvements Jul 11, 2017
…when appropriate

Previously a CFI anchor was generated with a 1 character offset:
First: `/4/2[introduction]/2,/1:0,/1:1`
Last: `/4/2[introduction]/14,/1:223,/1:224`

Now this anchor is still a single character but the start and end being the same start offset, hence a collapsed range or “caret” range
First: `/4/2[introduction]/2/1:0`
Last: `/4/2[introduction]/14/1:224`
@danielweck danielweck merged commit 033d256 into readium:develop Jul 13, 2017
@jccr jccr deleted the feature/improved-cfi-logic branch June 20, 2019 20:50
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.

5 participants