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.

Sign up for GitHub

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

Jump to bottom

[IP-925]: Preview pdf in modal #944

Draft
wants to merge 16 commits into
base: development
Choose a base branch
from
Draft

[IP-925]: Preview pdf in modal #944

wants to merge 16 commits into from

Conversation

Verony-makesIT
Copy link
Contributor

Description

For those who prefer to view PDF's in a Modal instead of an additional TAB window. Switching back to the old default remains possible. The option can be changed via the setting "General" - "Show Options menu item [Preview PDF]".

  • By default the setting is set to "No" so that the Options menu item "Download PDF" is shown and the PDF of the invoice/quote will be displayed in a TAB window.
  • Setting the option to "Yes" shows the Options menu item "Preview PDF" and will display the PDF of the invoice/quote in a Modal window.

Related Issue

#925

Motivation and Context

This option gives the user the choice of viewing the PDF of an invoice or quote, either in a TAB window (= default) or via a Modal window.

Screenshots (if appropriate):

Pull Request Checklist

  • My code follows the code formatting guidelines.
  • I have an issue ID for this pull request.
  • I selected the corresponding branch.
  • I have rebased my changes on top of the corresponding branch.

Issue Type (Please check one or more)

  • Bugfix
  • Improvement of an existing Feature
  • New Feature

@Verony-makesIT
small format indent issue
8e6579a
@Verony-makesIT
Added setting and Options menu-item [Preview PDF]
cf35df4
@Verony-makesIT
Small format indent issue
7cde52c
@Verony-makesIT
Added code to: Preview quote/invoice pdf in modal window
148a922 
Added code to implement the feature: Show quote/invoice pdf in modal if Options menu setting 'Preview PDF' is active.
@Verony-makesIT
Added code to: Preview quote/invoice pdf in modal window in table
70bac8c 
Added code to implement the feature: Show quote/invoice pdf in modal (for table view) if Options menu setting 'Preview PDF' is active.
@Verony-makesIT
Merge branch 'InvoicePlane:development' into Preview-PDF-in-Modal-Inv…
f6ca2df 
…oicePlane#925
@clockwiseq clockwiseq added Needs review Feature Request Improvement or Feature Request labels Apr 14, 2023
@clockwiseq clockwiseq linked an issue Apr 14, 2023 that may be closed by this pull request
[Feature Request] Add item "Preview PDF" to Options menu and show in a modal window #925
Open
clockwiseq
clockwiseq approved these changes Apr 14, 2023
View reviewed changes
Copy link
Contributor

@clockwiseq clockwiseq 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

@clockwiseq
Copy link
Contributor

Although I have reviewed the pr, I want @naui95 and @nielsdrost7 to take a look and to perform the merge.

@nielsdrost7 nielsdrost7 marked this pull request as draft April 14, 2023 18:53
@nielsdrost7
Copy link
Contributor

  • I converted the PR to `draft, so we don't merge it yet, until it's approved
  • Step 1 is to 'squash' your commits (or fixup, it's basically the same thing)
    git rebase -i (either f6ca2df or 70bac8c
    It will give you a prompt with all your commits and then you place a code in front of them what you would like to do.
    It's mostly fixup and the top line will stay on pick.

We'll guide you through this on slack.

Second thing:
We cannot merge this, until 1.6.1 has been reased

@nielsdrost7
Copy link
Contributor

I haven't looked into the PR, but are you introducing a setting? Does that mean a database change?

@Verony-makesIT
Copy link
Contributor Author

@nielsdrost7:
Regarding your "database change" question: When the Option is checked, only a new record (show_menu_item_preview_pdf) is added to the db table "ip_settings". That's all...

@Verony-makesIT
Copy link
Contributor Author

@nielsdrost7:
Aha, I think I understand why you are asking that (db change) question after rereading the last commit title. The title of commit "70bac8c" should be: Preview quote/invoice pdf in modal window (for table view) (= changes in php files partial_invoice/quote_table)

@nielsdrost7 nielsdrost7 changed the title Preview pdf in modal #925 [IP-925]: Preview pdf in modal Dec 23, 2023
nielsdrost7
nielsdrost7 requested changes May 16, 2024
View reviewed changes
application/modules/invoices/views/modal_preview_pdf.php
<h3><i class="fa fa-file-pdf-o"></i><?php echo ' ' . trans('invoice') . ': #' . $invoice->invoice_number ; ?></h3>
</div>
<div class="modal-body" style="padding: 0; margin: 0; background-color: #ededf0;">
<iframe id="iframe_pdf" src="<?php echo site_url('invoices/generate_pdf/' . $invoice_id); ?>#zoom=page-width" width="100%" height="70%" scrolling="no"
Copy link
Contributor

Choose a reason for hiding this comment

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

@Verony-makesIT can you find another solution than an iframe?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nielsdrost7, I assume your question is because of a possible xss security issue with iframes?
I am currently exploring other options such as embed or object.
If you think of other or better options for previewing PDFs, please let me know...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nielsdrost7 nielsdrost7 nielsdrost7 requested changes

@clockwiseq clockwiseq clockwiseq approved these changes

Requested changes must be addressed to merge this pull request.

Assignees

@naui95 naui95

@clockwiseq clockwiseq

@nielsdrost7 nielsdrost7

Labels
Feature Request Improvement or Feature Request Needs review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Feature Request] Add item "Preview PDF" to Options menu and show in a modal window
4 participants
@Verony-makesIT @clockwiseq @nielsdrost7 @naui95