Skip to content

feat(image): Add support for region annotations on image files#508

Merged
jstoffan merged 1 commit intobox:masterfrom
jstoffan:add-image-support
Jun 17, 2020
Merged

feat(image): Add support for region annotations on image files#508
jstoffan merged 1 commit intobox:masterfrom
jstoffan:add-image-support

Conversation

@jstoffan
Copy link
Collaborator

@jstoffan jstoffan commented Jun 5, 2020

This solution will correctly display rotated annotations when the underlying image is rotated. However, creating annotations when the image is rotated will lead to them being mispositioned. Supporting that case is more complex, so I'll tackle it in a follow-up patchset.

Todo

  • Cross-browser testing
  • Unit tests for ImageAnnotator

}

this.getManagers(this.annotatedEl, referenceEl).forEach(manager => {
manager.style({
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This feels a little sketchy to me and can probably be refined. Unfortunately, we're not able to rely on the position/size of a parent element for images, as the img tag itself is sized directly by the Preview SDK. This solution keeps the annotation rendering area (ba-Layer) in sync with the image element.

@jstoffan jstoffan marked this pull request as ready for review June 6, 2020 00:35
@jstoffan jstoffan requested a review from a team as a code owner June 6, 2020 00:35

export default class RegionManager implements BaseManager {
page: number;
location: number;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm trying to de-emphasize the concept of pages within the annotator components. We may want to use these on non-paged files eventually.

@jstoffan jstoffan force-pushed the add-image-support branch from 6c23e6e to f7635b9 Compare June 6, 2020 00:48
ConradJChan
ConradJChan previously approved these changes Jun 9, 2020
Copy link
Contributor

@ConradJChan ConradJChan left a comment

Choose a reason for hiding this comment

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

LGTM

mickr
mickr previously approved these changes Jun 17, 2020
Copy link
Collaborator

@mickr mickr left a comment

Choose a reason for hiding this comment

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

:shipit: left a few comments but overall looks great!

@jstoffan jstoffan merged commit 5ba56de into box:master Jun 17, 2020
@jstoffan jstoffan deleted the add-image-support branch June 17, 2020 21:37
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.

3 participants