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

[iOS/Android] Add config tools for checkmark, crossmark, dot #672

Merged
merged 21 commits into from
Dec 15, 2023

Conversation

darrenchann
Copy link
Contributor

@darrenchann darrenchann commented Dec 12, 2023

add Config.Tools.annotationCreateCheckMarkStamp, Config.Tools.annotationCreateCrossMarkStamp, Config.Tools.annotationCreateDotStamp to android/ios

for testing:

const myToolbar = {
      [Config.CustomToolbarKey.Id]: 'myToolbar',
      [Config.CustomToolbarKey.Name]: 'myToolbar', 
      [Config.CustomToolbarKey.Icon]: Config.ToolbarIcons.FillAndSign,
      [Config.CustomToolbarKey.Items]: [Config.Tools.annotationCreateCheckMarkStamp, Config.Tools.annotationCreateArrow, Config.Tools.annotationCreateDotStamp, Config.Tools.annotationCreateCallout]
    };
    <DocumentView
    annotationToolbars={[Config.DefaultToolbars.Annotate, myToolbar]}
    />

@ama-pdftron
Copy link
Contributor

You will need to manually update package.json:
64b9aaa

@darrenchann
Copy link
Contributor Author

updated package.json increment

Copy link
Contributor

@JamieDass JamieDass left a comment

Choose a reason for hiding this comment

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

The functionality works but I think the naming is a bit inconsistent with other annotation types.
I think it would be more consistent if we did:
FormFillCheck -> AnnotationCreateCheckMarkStamp
FormFillCross -> AnnotationCreateCrossMarkStamp
FormFillDot -> AnnotationCreateDotStamp

and same for the keys in the Config file
formFillCheck -> annotationCreateCheckMarkStamp
formFillCross -> annotationCreateCrossMarkStamp
formFillDot -> annotationCreateDotStamp

@brandenfung2 what are your thoughts on that?

@darrenchann
Copy link
Contributor Author

The functionality works but I think the naming is a bit inconsistent with other annotation types. I think it would be more consistent if we did: FormFillCheck -> AnnotationCreateCheckMarkStamp FormFillCross -> AnnotationCreateCrossMarkStamp FormFillDot -> AnnotationCreateDotStamp

and same for the keys in the Config file formFillCheck -> annotationCreateCheckMarkStamp formFillCross -> annotationCreateCrossMarkStamp formFillDot -> annotationCreateDotStamp

@brandenfung2 what are your thoughts on that?

I went for the suggested change for the names of the configs.

Copy link
Contributor

@JamieDass JamieDass left a comment

Choose a reason for hiding this comment

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

Looks good to me, need to wait for Android review to merge.

Copy link
Contributor

@brandenfung2 brandenfung2 left a comment

Choose a reason for hiding this comment

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

This does not work on Android:

disabledTools={[Config.Tools.annotationCreateCrossMarkStamp, Config.Tools.annotationCreateCheckMarkStamp, Config.Tools.annotationCreateDotStamp]}

Copy link
Contributor

@JamieDass JamieDass left a comment

Choose a reason for hiding this comment

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

Good catch by Branden.
setToolMode and disabledTools don't seem to work on iOS.

this._viewer.setToolMode(Config.Tools.annotationCreateCrossMarkStamp).then(() => {
      // done switching tools                                                                                                                                                                                                                                                                                                                                       
});
disabledTools={[Config.Tools.annotationCreateCrossMarkStamp, Config.Tools.annotationCreateCheckMarkStamp, Config.Tools.annotationCreateDotStamp]}

Copy link
Contributor

@JamieDass JamieDass left a comment

Choose a reason for hiding this comment

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

setToolMode is working on iOS.
Currently it is not possible to disable these tools through native APIs so we have added a ticket to our backlog to implement that and once that is done we can add that to the React Native wrapper as well.

@brandenfung2
Copy link
Contributor

Added support for setTool and disableTool for Dot, Checkmark, and Cross on Android

@brandenfung2 brandenfung2 merged commit 76ff9cd into master Dec 15, 2023
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.

4 participants