Skip to content

Conversation

@radazzouz
Copy link
Contributor

@radazzouz radazzouz commented Jan 7, 2019

⚠️ Do not merge yet! This PR requires PSPDFKit 8.1.3 to be released first. ⚠️


Details

This is an internal change. It does not impact the React Native JavaScript API. Internally, we are now using the annotation uuid property to remove existing annotation instead of the annotation name property.

Acceptance Criteria

  • The new implementation uses the annotation uuid property to remove existing annotation instead of the annotation name property.
  • The Catalog example "Programmatic Annotations" on iOS works.
  • The Catalog example "Programmatic Annotations" on Android works.
  • The Catalog example "Programmatic Annotations" on Windows works.

Copy link
Contributor

@nickwinder nickwinder left a comment

Choose a reason for hiding this comment

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

Windows does not have a remove Annotation yet. Instant features are handled in the SDK so no handling is needed in react-native wrapper.

Copy link
Contributor

@steviki steviki left a comment

Choose a reason for hiding this comment

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

LGTM 👍

steviki and others added 3 commits January 8, 2019 09:13
Co-Authored-By: radazzouz <radazzouz@users.noreply.github.com>
Co-Authored-By: radazzouz <radazzouz@users.noreply.github.com>
Copy link
Contributor

@irgendeinich irgendeinich left a comment

Choose a reason for hiding this comment

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

LGTM 👍
Might make sense to also do it this way on Android, though we would need to expose the UUID first.

@radazzouz radazzouz merged commit 0b39ad6 into master Jan 15, 2019
@radazzouz radazzouz deleted the use-uuids-for-removal branch January 15, 2019 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants