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

Include uuid in annotation removed event #349

Merged
merged 4 commits into from
Feb 12, 2020

Conversation

irgendeinich
Copy link
Contributor

@irgendeinich irgendeinich commented Feb 10, 2020

For #283

Details

  • Includes the uuid in the removed event.

Acceptance Criteria

  • When approved, right before merging, rebase with master and increment the package version in package.json, package-lock.json, samples/Catalog/package.json, and samples/NativeCatalog/package.json (see example commit: 1bf805f).
  • Create a new release (and tag) with the new package version (see https://github.com/PSPDFKit/react-native/releases).

@@ -57,6 +57,7 @@ public void dispatch(RCTEventEmitter rctEventEmitter) {
annotationMap = new HashMap<>();
annotationMap.put("name", annotation.getName());
annotationMap.put("creatorName", annotation.getCreator());
Copy link
Contributor

Choose a reason for hiding this comment

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

On iOS, we are not including the creator's name when an annotation is deleted:

[annotationsJSON addObject:@{@"uuid" : annotation.uuid, @"name" : annotation.name ?: [NSNull null]}];

The change would be pretty trivial:

- [annotationsJSON addObject:@{@"uuid" : annotation.uuid, @"name" : annotation.name ?: [NSNull null]}];
+ [annotationsJSON addObject:@{@"uuid" : annotation.uuid, @"name" : annotation.name ?: [NSNull null], @"creatorName" : annotation.user ?: [NSNull null]}];

@davidschreiber can you please make this change in this PR? Otherwise, I can do it myself in the PR or a followup PR.

Pinging @nickwinder, as I am not sure what Windows does when an annotation is deleted.

@radazzouz radazzouz self-requested a review February 11, 2020 16:08
Copy link
Contributor

@radazzouz radazzouz left a comment

Choose a reason for hiding this comment

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

LGTM 👍 Thanks for making the iOS change, @irgendeinich!

@irgendeinich irgendeinich merged commit 9c065e5 into master Feb 12, 2020
@irgendeinich irgendeinich deleted the reinhard/update-on-annotations-changed branch February 12, 2020 12:44
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.

4 participants