PDF annotation support #787

gpsrenewablesit · 2023-07-19T09:25:27Z

I have enabled the annotation toolbar within the PDF viewer interface. This toolbar includes various annotation tools, such as freehand drawing and text boxes. Clicking the save button will save the updated PDF document to Nextcloud.

After the update

Changes after update

Enabled text annotation.
Enabled freehand drawing.
Enabled save button.
Saving the updated PDF to Nextcloud.

Similar Issues

Close #719
Close #647

szaimen

I tested this and it works quite well. Thanks a lot for you contribution!

However from design perspective, this description is not really visible.

Also please make sure to translate all text that you added. See https://docs.nextcloud.com/server/latest/developer_manual/basics/front-end/l10n.html#javascript

appinfo/info.xml

szaimen · 2023-07-19T11:32:09Z

Also @gpsrenewablesit can you please fix DCO, see https://github.com/nextcloud/files_pdfviewer/pull/787/checks?check_run_id=15161520585. Otherwise we cannot merge this. Thank you! :)

github-actions · 2023-08-03T02:08:31Z

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the reviewing process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR reviewing process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

szaimen

Works well in my testing, thank you! :)

nopper · 2023-10-07T13:06:42Z

What's the state of this? Can we get this approved?

szaimen · 2023-10-07T13:41:03Z

What's the state of this? Can we get this approved?

@danxuliu still needs to review...

danxuliu

Sorry for the very long delay and thanks for your contribution!

In order to ask for some changes in the review I had to implement them by myself, as I was not sure if/how they could be done (and some were quite tricky...). However, as I had already implemented them for testing purposes I cleaned up my own code and ended commiting the changes too rather than just asking for them :-)

Due to that I also took the freedom to rework the commit history rather than asking for that (as, for example, the commits that fixed the minimum version or the translated strings should be squashed with the original commit, and the upload code could be set in the component since the beginning instead of moving it from the worker in an additional commit); I hope that is fine for you, but please accept my apologies if not.

Besides that, I have rebased the pull request on latest master to solve the conflicts.

I will now force push the branch with those changes.

templates/viewer.php

src/workersrc.js

templates/viewer.php

src/workersrc.js

danxuliu · 2023-10-09T20:40:54Z

There is something I would like to ask @nextcloud/designers about :-) In PDF.js UI the Save button (which has a Download icon) always downloads the PDF file. Here, on the other hand, the Save button uploads the changed file with the annotations to Nextcloud. This could be a surprising behaviour (specially for Firefox users, as Firefox uses PDF.js to show PDF files). Should a different button, maybe one with an upload icon, be used instead? Should Save still be used as caption?

And if an additional button to upload the file is used, should the download button be shown as well with its regular behaviour? Before this pull request the Save button was always hidden (as the file could be saved/downloaded using the button in the top bar from the viewer or the public page, depending on the case). However, if annotations are made and the user downloads the file through those means the downloaded file would not contain the annotations, it would still be the original file in the server. The user would need to first upload it and, then, download it to get the annotations. Which, obviously, changes the original file in the server and may not be desirable 🤷

On the other hand... if the download button inside the PDF viewer behaves differently than the download button in the file list or the public page buttons that could be confusing as well (and intercepting the clicks (or any other activation) on the top bar buttons to download the annotated file instead of the original file while not modifying the file in the server could be difficult, although I have not actually checked).

Thanks!

szaimen · 2023-10-18T10:21:32Z

There is something I would like to ask @nextcloud/designers about :-) In PDF.js UI the Save button (which has a Download icon) always downloads the PDF file. Here, on the other hand, the Save button uploads the changed file with the annotations to Nextcloud. This could be a surprising behaviour (specially for Firefox users, as Firefox uses PDF.js to show PDF files). Should a different button, maybe one with an upload icon, be used instead? Should Save still be used as caption?

And if an additional button to upload the file is used, should the download button be shown as well with its regular behaviour? Before this pull request the Save button was always hidden (as the file could be saved/downloaded using the button in the top bar from the viewer or the public page, depending on the case). However, if annotations are made and the user downloads the file through those means the downloaded file would not contain the annotations, it would still be the original file in the server. The user would need to first upload it and, then, download it to get the annotations. Which, obviously, changes the original file in the server and may not be desirable 🤷

On the other hand... if the download button inside the PDF viewer behaves differently than the download button in the file list or the public page buttons that could be confusing as well (and intercepting the clicks (or any other activation) on the top bar buttons to download the annotated file instead of the original file while not modifying the file in the server could be difficult, although I have not actually checked).

I'd say that we should change the save icon to a normal save icon and keep the caption (or maybe make it more explicit and change that to Save to Nextcloud.) Then we should be fine :)

szaimen

@danxuliu I tested this and it works fine generally but I don't know why and if it is caused by this PR but somehow the pdf viewer has two scrollbars now:

The Viewer provides several useful helper functions for DAV, but they can not be used (or at least I did not find out how) from the PDF viewer. Therefore, for now they were just copied over. The original file is written in TypeScript, but as it was not extensively typed and TypeScript is not currently supported in the PDF viewer for simplicity it was adjusted and converted to JavaScript. The file was copied from "src/utils/davUtils.ts" of repository "https://github.com/nextcloud/viewer" at commit "854e63d33a8fd304a6c4ea6704f0f68c6308f4b8". Signed-off-by: Daniel Calviño Sánchez <[email protected]>

Signed-off-by: Daniel Calviño Sánchez <[email protected]>

Signed-off-by: Mishel Shaji <[email protected]>

In Nextcloud the background of disabled buttons does not change when hovered. Signed-off-by: Daniel Calviño Sánchez <[email protected]>

Signed-off-by: Daniel Calviño Sánchez <[email protected]>

The "Save" button was always enabled, but saving and uploading the file would be needed only if there is an unsaved annotation. Therefore now the button is initially disabled, and it is enabled only when an annotation is added; once the file is saved the button is disabled and it will be enabled again once a new annotation is added. Signed-off-by: Daniel Calviño Sánchez <[email protected]>

The "Download / Save" button uses a "Download" icon by default. For clarity, and to differentiate from the default PDF.js behaviour that downloads the file with the added annotations rather than saving it in place, the button now uses a "Save" icon instead. Like other icons used in Nextcloud, the "Save" icon was copied from Material Icons, which are licensed under the Apache License Version 2.0: https://fonts.google.com/icons?selected=Material+Icons:save: Signed-off-by: Daniel Calviño Sánchez <[email protected]>

danxuliu · 2023-10-23T05:14:20Z

I'd say that we should change the save icon to a normal save icon and keep the caption (or maybe make it more explicit and change that to Save to Nextcloud.) Then we should be fine :)

I have replaced the default "Download" icon with a "Save" icon from Material Icons. If the button title was changed it should be "Save to XXX", where XXX is the Nextcloud name configured in the theme. That is probably easy to do, but... something for the future ;-) (but feel free to change that if you know how to, of course :-) ).

I tested this and it works fine generally but I don't know why and if it is caused by this PR but somehow the pdf viewer has two scrollbars now:

I have not been able to reproduce neither on this branch nor in master 🤷 Could you check if you also have that in master? If you do then it would be unrelated to this pull request, so please merge if the icon change is fine (I would like to avoid another rebase due to conflicts caused by dependency updates :-P ).

szaimen

I'd say that we should change the save icon to a normal save icon and keep the caption (or maybe make it more explicit and change that to Save to Nextcloud.) Then we should be fine :)

I have replaced the default "Download" icon with a "Save" icon from Material Icons. If the button title was changed it should be "Save to XXX", where XXX is the Nextcloud name configured in the theme. That is probably easy to do, but... something for the future ;-) (but feel free to change that if you know how to, of course :-) ).

Tested and looks good to me :)

I tested this and it works fine generally but I don't know why and if it is caused by this PR but somehow the pdf viewer has two scrollbars now:

I have not been able to reproduce neither on this branch nor in master 🤷 Could you check if you also have that in master? If you do then it would be unrelated to this pull request, so please merge if the icon change is fine (I would like to avoid another rebase due to conflicts caused by dependency updates :-P ).

Yes, could also reproduce on master. So merging 👍

szaimen · 2023-10-25T14:01:23Z

🎉🎉🎉🎉🎉

mkrecek234 · 2023-10-27T18:41:18Z

@danxuliu @szaimen When is this going to be released to Nextcloud app store? Would love to use it. Am currently on 2.8.0

szaimen · 2023-10-27T19:26:35Z

It will be released with Nextcloud 28 (Hub 7)

mkrecek234 · 2023-10-27T19:36:27Z

@szaimen You mean 28? We are on 27.1.3 already. Would love to see it as an app update already in NC27

szaimen · 2023-10-27T19:42:14Z

Sorry, made a typo. Meant 28 (hub 7)

gpsrenewablesit requested review from danxuliu and szaimen as code owners July 19, 2023 09:25

szaimen requested changes Jul 19, 2023

View reviewed changes

appinfo/info.xml Outdated Show resolved Hide resolved

szaimen added enhancement 3. to review labels Jul 19, 2023

szaimen added this to the Nextcloud 28 milestone Jul 19, 2023

gpsrenewablesit force-pushed the files-pdf-annotation branch from 0480aee to 07f2d25 Compare July 20, 2023 14:22

gpsrenewablesit requested a review from szaimen August 1, 2023 07:27

github-actions bot added the feedback-requested label Aug 3, 2023

szaimen approved these changes Aug 9, 2023

View reviewed changes

szaimen requested review from danxuliu and removed request for danxuliu August 9, 2023 10:22

szaimen mentioned this pull request Aug 27, 2023

chore(deps): Bump pdfjs-dist from 3.9.179 to 3.10.111 #814

Closed

danxuliu reviewed Oct 9, 2023

View reviewed changes

danxuliu force-pushed the files-pdf-annotation branch from 7be9854 to bc72b57 Compare October 9, 2023 20:37

danxuliu force-pushed the files-pdf-annotation branch from bc72b57 to ab4351b Compare October 9, 2023 21:06

szaimen reviewed Oct 18, 2023

View reviewed changes

danxuliu and others added 7 commits October 23, 2023 06:44

Add reference to PDFViewerApplication in PDFView component

b5709f7

Signed-off-by: Daniel Calviño Sánchez <[email protected]>

Added file annotation support

f4417bb

Signed-off-by: Mishel Shaji <[email protected]>

Do not highlight disabled toolbar buttons

8a82a80

In Nextcloud the background of disabled buttons does not change when hovered. Signed-off-by: Daniel Calviño Sánchez <[email protected]>

Show a loading spinner while the file is being uploaded

17a3550

Signed-off-by: Daniel Calviño Sánchez <[email protected]>

danxuliu force-pushed the files-pdf-annotation branch from f914dca to 78b6fbe Compare October 23, 2023 05:11

szaimen approved these changes Oct 25, 2023

View reviewed changes

szaimen merged commit c5c920d into nextcloud:master Oct 25, 2023
27 checks passed

szaimen mentioned this pull request Oct 25, 2023

PDF-viewer not full-screen anymore on NC28 #843

Closed

This was referenced Nov 3, 2023

[stable27] Show forms as read only #849

Merged

Editing PDF forms (and saving the results back to Nextcloud)? #354

Closed

Show forms as read-only if the file is not editable #850

Merged

mkrecek234 mentioned this pull request Aug 8, 2024

PDF Annotation - Add Signatures not as Annotations but fixed, non-removable changes #1004

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PDF annotation support #787

PDF annotation support #787

gpsrenewablesit commented Jul 19, 2023 •

edited by szaimen

Loading

szaimen left a comment

szaimen commented Jul 19, 2023

github-actions bot commented Aug 3, 2023

szaimen left a comment

nopper commented Oct 7, 2023

szaimen commented Oct 7, 2023

danxuliu left a comment

danxuliu commented Oct 9, 2023

szaimen commented Oct 18, 2023

szaimen left a comment

danxuliu commented Oct 23, 2023

szaimen left a comment

szaimen commented Oct 25, 2023

mkrecek234 commented Oct 27, 2023

szaimen commented Oct 27, 2023 •

edited

Loading

mkrecek234 commented Oct 27, 2023 •

edited

Loading

szaimen commented Oct 27, 2023

PDF annotation support #787

PDF annotation support #787

Conversation

gpsrenewablesit commented Jul 19, 2023 • edited by szaimen Loading

After the update

Changes after update

Similar Issues

szaimen left a comment

Choose a reason for hiding this comment

szaimen commented Jul 19, 2023

github-actions bot commented Aug 3, 2023

szaimen left a comment

Choose a reason for hiding this comment

nopper commented Oct 7, 2023

szaimen commented Oct 7, 2023

danxuliu left a comment

Choose a reason for hiding this comment

danxuliu commented Oct 9, 2023

szaimen commented Oct 18, 2023

szaimen left a comment

Choose a reason for hiding this comment

danxuliu commented Oct 23, 2023

szaimen left a comment

Choose a reason for hiding this comment

szaimen commented Oct 25, 2023

mkrecek234 commented Oct 27, 2023

szaimen commented Oct 27, 2023 • edited Loading

mkrecek234 commented Oct 27, 2023 • edited Loading

szaimen commented Oct 27, 2023

gpsrenewablesit commented Jul 19, 2023 •

edited by szaimen

Loading

szaimen commented Oct 27, 2023 •

edited

Loading

mkrecek234 commented Oct 27, 2023 •

edited

Loading