-
-
Notifications
You must be signed in to change notification settings - Fork 152
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
POS: Receipt Printer #1964
POS: Receipt Printer #1964
Conversation
I wonder if it's worth distinguishing |
Do you mean have a |
Precisely, and perhaps we add the payment pre-image on the receipt to distinguish them and act as a bit of payment proof |
Another nit: the header. Instead of |
|
package.json
Outdated
@@ -111,6 +111,7 @@ | |||
"react-native-notifications": "5.1.0", | |||
"react-native-os": "aprock/react-native-os#5/head", | |||
"react-native-permissions": "3.8.0", | |||
"react-native-print": "^0.11.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's lock in the version number here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
views/Order.tsx
Outdated
? merchantName | ||
: isPaid | ||
? 'Tax Receipt' | ||
: 'Invoice'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need these titles localized
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
Nice job only showing the printer setting for Android, but I think it would be better to make it opt in rather than opt out as most merchants likely won't have a printer module. |
Makes total sense. Flipped the setting from disable to enable. |
locales/en.json
Outdated
"pos.views.Settings.PointOfSale.authWarning": "Warning: no password or PIN set", | ||
"pos.views.Settings.PointOfSale.backendWarning": "Warning: currently only LND nodes are able to mark orders as paid", | ||
"pos.views.Settings.PointOfSale.currencyError": "Error: currency must be set first", | ||
"pos.print.taxReceipt": "Tax Receipt", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: can we just make this Receipt
instead of Tax Receipt
@Talej can you provide an example of what these receipts look like when the merchant provides their store name in settings? |
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, got it! I managed to pinpoint the issue on my end, and now I'm getting the option to download PDFs. Thanks! |
@@ -116,7 +116,8 @@ export default class OrderView extends React.Component<OrderProps, OrderState> { | |||
const { changeUnits, units } = UnitsStore; | |||
const fiat = settings.fiat; | |||
const disableTips: boolean = settings?.pos?.disableTips || false; | |||
const enablePrinter: boolean = settings?.pos?.enablePrinter || false; | |||
const enablePrinter: boolean = | |||
(settings?.pos?.enablePrinter && RNPrint) || false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess we can remove this condition now, so the user can see the PRINT button even if the printer is not connected and use it to download the receipt.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
possibly can't hurt to leave it in there just in case
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tACK
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Great job, @Talej!
Description
Receipt printing functionality for android. Ended up using react-native-print package with a HTML template for receipt layout. The layout should pretty closely match the details shown on the 'Order' view for a paid order.
A print receipt button is shown on the payment completed view and also on the order view for paid orders if:
react-native-print doesn't have any way of detecting if there is a printer ahead of calling the print dialog so to handle this I added the disable printer option under the POS settings.
I thought it could possibly also be useful to allow the receipt template HTML to be configuration in the POS settings - although given it is HTML this could be somewhat painful the edit on a device so wanted your input on this...
This pull request is categorized as a:
Checklist
yarn run tsc
and made sure my code compiles correctlyyarn run lint
and made sure my code didn’t contain any problematic patternsyarn run prettier
and made sure my code is formatted correctlyyarn run test
and made sure all of the tests passTesting
If you modified or added a utility file, did you add new unit tests?
I have tested this PR on the following platforms (please specify OS version and phone model/VM):
I have tested this PR with the following types of nodes (please specify node version and API version where appropriate):
Locales
Third Party Dependencies and Packages
yarn
after this PR is merged inpackage.json
andyarn.lock
have been properly updatedOther: