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.

Sign up for GitHub

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

Jump to bottom

feat(highlights): Render highlight annotations #550

Merged

ConradJChan merged 5 commits into box:master from ConradJChan:render-highlights

Aug 7, 2020

Contributor

ConradJChan commented Aug 3, 2020 •

edited

Loading

Screenshot:

TODO:

unit tests
cross browser testing


          feat(highlights): Render highlight annotations

5a9c734

ChenCodes reviewed

View reviewed changes

src/highlight/HighlightAnnotation.scss Outdated Show resolved Hide resolved

ChenCodes reviewed

View reviewed changes

src/highlight/HighlightAnnotation.tsx Show resolved Hide resolved

ChenCodes reviewed

View reviewed changes

src/highlight/highlightUtil.ts Outdated Show resolved Hide resolved

ChenCodes reviewed

View reviewed changes

src/__tests__/BoxAnnotations-test.js Show resolved Hide resolved

ChenCodes reviewed

View reviewed changes

src/highlight/HighlightContainer.tsx Outdated Show resolved Hide resolved

mxiao6 reviewed

View reviewed changes

src/highlight/HighlightAnnotation.tsx Outdated Show resolved Hide resolved

src/highlight/HighlightAnnotation.tsx Outdated Show resolved Hide resolved

src/highlight/HighlightAnnotation.tsx Outdated Show resolved Hide resolved

src/highlight/HighlightList.tsx Outdated Show resolved Hide resolved

Contributor

ChenCodes commented Aug 4, 2020

Also in the screenshot it looks like the highlighted portion is little bit offset (it looks like it's higher than it should be). Will we eventually address this?

mxiao6 reviewed

View reviewed changes

src/highlight/HighlightList.tsx Outdated Show resolved Hide resolved


          chore: pr comments and x-browser compat

d7e422f

Contributor Author

ConradJChan commented Aug 5, 2020

@ChenCodes yeah, unfortunately, this is a longstanding issue with PDFJS: mozilla/pdf.js#9316

ConradJChan commented

View reviewed changes

src/document/DocumentAnnotator.ts Show resolved Hide resolved


          chore: unit tests

813ab6f

ConradJChan marked this pull request as ready for review

August 5, 2020 22:37

ConradJChan requested a review from a team as a code owner

August 5, 2020 22:37

ConradJChan force-pushed the render-highlights branch from 18ef97a to 0ba0438 Compare

August 6, 2020 07:28


          chore: add HighlightCanvas

fe27d43

ConradJChan force-pushed the render-highlights branch from 0ba0438 to fe27d43 Compare

August 7, 2020 17:12

Contributor

mxiao6 commented Aug 7, 2020

Do we need to add FF guard for the new layers?

mxiao6 reviewed

View reviewed changes

src/highlight/HighlightCanvas.tsx Outdated Show resolved Hide resolved

src/highlight/HighlightAnnotation.tsx Show resolved Hide resolved

src/highlight/__tests__/HighlightAnnotation-test.tsx Outdated Show resolved Hide resolved

Contributor Author

ConradJChan commented Aug 7, 2020

@mxiao6 I think FF isn't needed because creation is disabled so there should be no impact. Let me know if you disagree


          chore: pr comments

ConradJChan force-pushed the render-highlights branch from bcdeb45 to 1830867 Compare

August 7, 2020 22:09

mxiao6 approved these changes

View reviewed changes

ConradJChan merged commit 3586412 into box:master

ConradJChan deleted the render-highlights branch

August 7, 2020 22:33

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet