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

Standalone POS #1722

Merged
merged 17 commits into from Dec 28, 2023
Merged

Standalone POS #1722

merged 17 commits into from Dec 28, 2023

Conversation

Talej
Copy link
Contributor

@Talej Talej commented Oct 2, 2023

Description

Implements standalone POS functionality.

  • Changes the enable square POS setting to allow enable Square or Standalone mode.
  • Under POS settings adds support for managing categories and products.
  • Products can be priced in sats or fiat.
  • Adds a standalone POS order creation screen and allows invoicing through the existing payment flow.

** Note: ** I haven't yet tested this on an Android device. Currently without a working device

This pull request is categorized as a:

  • New feature
  • Bug fix
  • Code refactor
  • Configuration change
  • Locales update
  • Quality assurance
  • Other

Checklist

  • I’ve run yarn run tsc and made sure my code compiles correctly
  • I’ve run yarn run lint and made sure my code didn’t contain any problematic patterns
  • I’ve run yarn run prettier and made sure my code is formatted correctly
  • I’ve run yarn run test and made sure all of the tests pass

Testing

If you modified or added a utility file, did you add new unit tests?

  • No, I’m a fool
  • Yes
  • N/A

I have tested this PR on the following platforms (please specify OS version and phone model/VM):

  • Android
  • iOS

I have tested this PR with the following types of nodes (please specify node version and API version where appropriate):

  • Embedded LND
  • LND (REST)
  • LND (Lightning Node Connect)
  • Core Lightning (c-lightning-REST)
  • LndHub
  • [DEPRECATED] Core Lightning (Spark)
  • [DEPRECATED] Eclair

Locales

  • I’ve added new locale text that requires translations
  • I’m aware that new translations should be made on the Zeus Transfix page and not directly to this repo

Third Party Dependencies and Packages

  • Contributors will need to run yarn after this PR is merged in
  • 3rd party dependencies have been modified:
    • verify that package.json and yarn.lock have been properly updated
    • verify that dependencies are installed for both iOS and Android platforms

Other:

  • Changes were made that require an update to the README
  • Changes were made that require an update to onboarding

@Talej Talej changed the base branch from pos to master October 2, 2023 23:17
@kaloudis kaloudis added this to the v0.8.1 milestone Oct 12, 2023
Copy link
Contributor

@shubhamkmr04 shubhamkmr04 left a comment

Choose a reason for hiding this comment

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

Simulator Screenshot - iPhone 14 Plus - 2023-10-12 at 18 45 54
Getting this warning as soon as we open the POS settings

Copy link
Contributor

@shubhamkmr04 shubhamkmr04 left a comment

Choose a reason for hiding this comment

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

Screen.Recording.2023-10-12.at.6.59.19.PM.mov

Not able to change the unit to 'sats'

Copy link
Contributor

@shubhamkmr04 shubhamkmr04 left a comment

Choose a reason for hiding this comment

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

If the Category gets deleted, the product belonging to that Category should also be deleted.

Copy link
Contributor

@shubhamkmr04 shubhamkmr04 left a comment

Choose a reason for hiding this comment

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

Screen.Recording.2023-10-12.at.10.08.11.PM.mov

We should have a counter here on this view to know how many products we have selected so far and maybe a way to remove it, too.
@kaloudis, what do you think about this?

Copy link
Contributor

@shubhamkmr04 shubhamkmr04 left a comment

Choose a reason for hiding this comment

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

Screen.Recording.2023-10-12.at.10.13.21.PM.mov

So we add our product to checkout and pay, then delete it from the settings, come to the homepage, and still can see our checked-out price belonging to our deleted product.

@kaloudis
Copy link
Contributor

Screen.Recording.2023-10-12.at.10.08.11.PM.mov

We should have a counter here on this view to know how many products we have selected so far and maybe a way to remove it, too. @kaloudis, what do you think about this?

+1 on having a counter

@kaloudis
Copy link
Contributor

Screen.Recording.2023-10-12.at.10.13.21.PM.mov

So we add our product to checkout and pay, then delete it from the settings, come to the homepage, and still can see our checked-out price belonging to our deleted product.

Deleting a product should probably trigger a cart clear

@Talej
Copy link
Contributor Author

Talej commented Oct 13, 2023

Screen.Recording.2023-10-12.at.10.08.11.PM.mov
We should have a counter here on this view to know how many products we have selected so far and maybe a way to remove it, too. @kaloudis, what do you think about this?

I have added an item counter in the "Charge" button.

Other thing I could think to do is add - (n) + in each of the product components. Might not be super user friendly though IMO.

@Talej
Copy link
Contributor Author

Talej commented Oct 13, 2023

Simulator Screenshot - iPhone 14 Plus - 2023-10-12 at 18 45 54 Getting this warning as soon as we open the POS settings

This one would've already been in there previously but I have sorted it out by dropping the FlatList

@Talej
Copy link
Contributor Author

Talej commented Oct 13, 2023

All of these items and a couple more things I found should be sorted out now.

@shubhamkmr04
Copy link
Contributor

Screen.Recording.2023-10-12.at.10.08.11.PM.mov
We should have a counter here on this view to know how many products we have selected so far and maybe a way to remove it, too. @kaloudis, what do you think about this?

I have added an item counter in the "Charge" button.

Other thing I could think to do is add - (n) + in each of the product components. Might not be super user friendly though IMO.

Yup, I understood. @kaloudis, should we do that, or the overall counter would be fine

Copy link
Contributor

@shubhamkmr04 shubhamkmr04 left a comment

Choose a reason for hiding this comment

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

Should we also remove open orders of deleted products?🤔

Copy link
Contributor

@shubhamkmr04 shubhamkmr04 left a comment

Choose a reason for hiding this comment

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

Screen.Recording.2023-10-13.at.1.21.27.PM.mov

We cannot save a product to the first category in the list; it goes into the uncategorized products. It only happens in the android.

Copy link
Contributor

@shubhamkmr04 shubhamkmr04 left a comment

Choose a reason for hiding this comment

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

On the ProductDetails and ProductCategoryDetails views, In the delete button, we should have our props containerStyle and titleStyle set to themeColor('delete') to maintain consistency with other views.

@shubhamkmr04
Copy link
Contributor

Screen.Recording.2023-10-12.at.10.13.21.PM.mov
So we add our product to checkout and pay, then delete it from the settings, come to the homepage, and still can see our checked-out price belonging to our deleted product.

Deleting a product should probably trigger a cart clear

Would it be better to subtract that deleted product's price instead of clearing the whole cart?

Copy link
Contributor

@shubhamkmr04 shubhamkmr04 left a comment

Choose a reason for hiding this comment

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

Screen.Recording.2023-10-13.at.1.55.07.PM.mov

Products belonging to the deleted category still exist. Only the last saved product gets deleted.

@Talej
Copy link
Contributor Author

Talej commented Oct 21, 2023

Screen.Recording.2023-10-12.at.10.13.21.PM.mov
So we add our product to checkout and pay, then delete it from the settings, come to the homepage, and still can see our checked-out price belonging to our deleted product.

Deleting a product should probably trigger a cart clear

Would it be better to subtract that deleted product's price instead of clearing the whole cart?

I think it would be a pretty rare condition that the user is editing/deleting products at the same time as actually making sales so I feel like just clearing the whole cart should be sufficient. What do you think?

@Talej
Copy link
Contributor Author

Talej commented Oct 21, 2023

Should we also remove open orders of deleted products?🤔

I feel like maybe this would lead to confusion and it should probably be left to the user to either hide/cancel the order or create a new one.

@Talej
Copy link
Contributor Author

Talej commented Oct 21, 2023

  • Fixed category delete issue with products
  • Fixed issue with selecting category for product on android
  • Updated styling for delete buttons

Ready for another round!

@kaloudis
Copy link
Contributor

Screen.Recording.2023-10-12.at.10.13.21.PM.mov
So we add our product to checkout and pay, then delete it from the settings, come to the homepage, and still can see our checked-out price belonging to our deleted product.

Deleting a product should probably trigger a cart clear

Would it be better to subtract that deleted product's price instead of clearing the whole cart?

I think it would be a pretty rare condition that the user is editing/deleting products at the same time as actually making sales so I feel like just clearing the whole cart should be sufficient. What do you think?

This should suffice

@kaloudis
Copy link
Contributor

Should we also remove open orders of deleted products?🤔

I feel like maybe this would lead to confusion and it should probably be left to the user to either hide/cancel the order or create a new one.

can go either way here. I don't think this is a deal breaker either way. maybe something that gets iterated on

Copy link
Contributor

@shubhamkmr04 shubhamkmr04 left a comment

Choose a reason for hiding this comment

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

Screen.Recording.2023-10-23.at.4.25.41.PM.mov

The screen returns to the top when I select any product, which might affect the user's experience as they would have to scroll down again and again if we have so many products listed.

}
];
categoryOptions = categoryOptions.concat(...mappedCategories);
console.log('options', categoryOptions);
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need this console, or was it just for debugging?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Was just for debugging. Have removed now

@Talej
Copy link
Contributor Author

Talej commented Oct 27, 2023

Screen.Recording.2023-10-23.at.4.25.41.PM.mov
The screen returns to the top when I select any product, which might affect the user's experience as they would have to scroll down again and again if we have so many products listed.

Having the Charge button up in the Tab Navigator was triggering re-render on every touch. I have restructured it so that doesn't happen any more.

Unfortunately means that I can't use the Tab Navigator space now so we've lost a bit of screen real estate

@shubhamkmr04
Copy link
Contributor

Simulator Screenshot - iPhone 14 Plus - 2023-10-27 at 19 09 20
@kaloudis, does the button look good in here?

Copy link
Contributor

@shubhamkmr04 shubhamkmr04 left a comment

Choose a reason for hiding this comment

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

Screen.Recording.2023-10-27.at.7.24.49.PM.mov

The amount is stuck on the price of first product I selected.

@Talej
Copy link
Contributor Author

Talej commented Oct 28, 2023

Screen.Recording.2023-10-27.at.7.24.49.PM.mov
The amount is stuck on the price of first product I selected.

Not sure how I missed that one. Nice catch. Fixed now!

@kaloudis
Copy link
Contributor

Screenshot 2023-12-18 at 19 42 36

hitting an issue here where the AmountInput component is pre-filled with value undefined and I cannot change the amount

@Talej
Copy link
Contributor Author

Talej commented Dec 19, 2023

Screenshot 2023-12-18 at 19 42 36 hitting an issue here where the `AmountInput` component is pre-filled with value `undefined` and I cannot change the amount

creating a new product or editing an existing?

@kaloudis
Copy link
Contributor

Screenshot 2023-12-18 at 19 42 36 hitting an issue here where the `AmountInput` component is pre-filled with value `undefined` and I cannot change the amount

creating a new product or editing an existing?

create a new product

@kaloudis
Copy link
Contributor

Looking good now! I've also double checked the structure to ensure that fiat prices are recorded properly.

Copy link
Contributor

@shubhamkmr04 shubhamkmr04 left a comment

Choose a reason for hiding this comment

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

The amount is prefilled with 0 that we can't remove

Screenshot 2023-12-20 at 12 11 14 PM Screenshot 2023-12-20 at 12 11 48 PM

@Talej
Copy link
Contributor Author

Talej commented Dec 20, 2023

The amount is prefilled with 0 that we can't remove

Screenshot 2023-12-20 at 12 11 14 PM Screenshot 2023-12-20 at 12 11 48 PM

Fixed

@shubhamkmr04
Copy link
Contributor

Looks fine now.

@kaloudis
Copy link
Contributor

Code looks good. A few things missing though:

  • Ability (or option) to go to order overview before generating invoice (allows user to set tip on this screen and see the total w/ tax) [you can go to this view if you don't pay and click on an order under the Open tab)
  • Ability to set tax %
  • Ability to disable tips

LMK what you think @Talej. Open to shipping it in beta1 as is and breaking up follow up work in other PRs.

@kaloudis
Copy link
Contributor

nit: let's also put the Standalone option above Square in the selection menu

@kaloudis
Copy link
Contributor

I've made the suggested changes myself to the PR:

  • Standalone placed above Square in the mode selection dropdown
  • Charging an order now takes you to an itemized breakdown instead of directly to the invoice (let's give some thought into if we want to make this configurable)
  • Disable tip and conf preferences are now displayed when on the POS settings when in Standalone mode
  • New tax percentage setting added to POS settings when in Standalone mode

I also noticed a bug with how itemized products were displayed on the Order view: amount displayed was 100x lower. Change can be found here: ad49272#diff-7ffe38a0457034ead72df14de5a05813a6c8a8945e64551040568d72e81a7d5eR330 Just need a sanity check here, but it appears to be rendering properly now.

@Talej
Copy link
Contributor Author

Talej commented Dec 27, 2023

I've made the suggested changes myself to the PR:

  • Standalone placed above Square in the mode selection dropdown
  • Charging an order now takes you to an itemized breakdown instead of directly to the invoice (let's give some thought into if we want to make this configurable)
  • Disable tip and conf preferences are now displayed when on the POS settings when in Standalone mode
  • New tax percentage setting added to POS settings when in Standalone mode

I also noticed a bug with how itemized products were displayed on the Order view: amount displayed was 100x lower. Change can be found here: ad49272#diff-7ffe38a0457034ead72df14de5a05813a6c8a8945e64551040568d72e81a7d5eR330 Just need a sanity check here, but it appears to be rendering properly now.

This all checks out and looks okay to me!

Copy link
Contributor

@shubhamkmr04 shubhamkmr04 left a comment

Choose a reason for hiding this comment

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

tACK

@kaloudis kaloudis merged commit 3c5ba98 into ZeusLN:master Dec 28, 2023
3 checks passed
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.

None yet

3 participants