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

Add ability to disable annotations (including bboxes in canvas) #963

Closed
2 tasks done
yuliya-ivaniukovich opened this issue Mar 14, 2022 · 4 comments · May be fixed by #968
Closed
2 tasks done

Add ability to disable annotations (including bboxes in canvas) #963

yuliya-ivaniukovich opened this issue Mar 14, 2022 · 4 comments · May be fixed by #968
Labels
enhancement New feature or request stale

Comments

@yuliya-ivaniukovich
Copy link

Before you start - checklist

  • I understand that React-PDF does not aim to be a fully-fledged PDF viewer and is only a tool to make one
  • I have checked if this feature request is not already reported

Description

At the moment it seems there is no way to pass ANNOTATION_MODE.DISABLED into render page canvas context:

annotationMode: renderForms ? ANNOTATION_MODE.ENABLE_FORMS : ANNOTATION_MODE.ENABLE,

We would like to have the ability to hide annotations at all, not only annotations layer.

Proposed solution

We can use existing renderAnnotationLayer prop to also hide annotations boxes during canvas rendering

Alternatives

Alternatively we can introduce a new property. That could be some specific like disableAnnotations or more generic like renderContext which can have arbitrary props that override the defaults.

Additional information

A bit later I'm going to propose a PR for this feature request, but would be great to discuss the preferable approach first

@yuliya-ivaniukovich yuliya-ivaniukovich added the enhancement New feature or request label Mar 14, 2022
@wojtekmaj
Copy link
Owner

wojtekmaj commented Mar 15, 2022

Hey @yuliya-ivaniukovich, I totally hear you!

Here's what I think:

There are a number of components depending on renderForms prop at the moment. Introducing a breaking change in this area would not be acceptable at the moment. What we can do though is introduce annotationMode prop which would give users greater control than renderForms flag.

We will need to do a better job at documenting things than Mozilla - annotationMode seems to be quite confusing if someone comes from renderForms approach.

PageCanvas needs an update in the line you've mentioned:

annotationMode: renderForms ? ANNOTATION_MODE.ENABLE_FORMS : ANNOTATION_MODE.ENABLE,

Something like

      annotationMode:
        annotationMode ?? (renderForms ? ANNOTATION_MODE.ENABLE_FORMS : ANNOTATION_MODE.ENABLE),

should do the trick!

Then, there's AnnotationLayer which actually uses renderForms flag:

const parameters = {
annotations,
div: this.annotationLayer,
imageResourcesPath,
linkService,
page,
renderForms,
viewport,
};
this.annotationLayer.innerHTML = '';
try {
pdfjs.AnnotationLayer.render(parameters);

Sooo maybe:

      renderForms:
        annotationMode !== null && annotationMode !== undefined
          ? annotationMode === ANNOTATION_MODE.ENABLE_FORMS
          : renderForms,

?

Then, in Page, defaultProps would need to change: renderForms default flag value must be removed, and default value for annotationMode (ANNOTATION_MODE.ENABLE) should be set instead. childContext needs to be updated to pass annotationMode.

In Test & LayerOptions, annotationMode controls must be added.

What do you think?

@yuliya-ivaniukovich
Copy link
Author

So, the idea is that passing annotationMode={ANNOTATION_FORMS.ENABLE_FORMS} should work the same way as passing renderForms. The proposal sounds totally reasonable, thanks.

@github-actions
Copy link
Contributor

This issue is stale because it has been open 90 days with no activity. Remove stale label or comment or this issue will be closed in 14 days.

@github-actions github-actions bot added the stale label Jun 20, 2022
@github-actions
Copy link
Contributor

This issue was closed because it has been stalled for 14 days with no activity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request stale
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants