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

Ensure "No" selection is maintained for custom price in POS app #2248

Merged
merged 1 commit into from Feb 10, 2021

Conversation

bolatovumar
Copy link
Contributor

@bolatovumar bolatovumar commented Feb 3, 2021

fix #2247

Screen.Recording.2021-02-02.at.8.15.56.PM.mov

@@ -210,7 +213,7 @@ $(function() {
image = productProperty.replace('image:', '').trim();
}
if (productProperty.indexOf('custom:') !== -1) {
custom =productProperty.replace('custom:', '').trim();
custom = productProperty.replace('custom:', '').trim();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just added a space here.

@@ -228,7 +231,7 @@ $(function() {
price: price,
image: image || null,
description: description || '',
custom: Boolean(custom),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since custom is always a string (from above custom = productProperty.replace('custom:', '').trim();) casting it to a boolean value using Boolean(custom) will always produce true unless custom happens to be an empty string. Checking for custom === "true" ensures the boolean value is set correctly.

@@ -263,8 +266,8 @@ $(function() {
if (inventory) {
template += ' inventory: ' + inventory + '\n';
}
if (custom) {
template += ' custom: true\n';
if (custom != null) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Make sure we don't just always set custom to true in the template.

@@ -376,6 +379,11 @@ $(function() {
return item;
},
unEscapeKey : function(k){
// Without this check a `false` boolean value will always be returned as an empty string
if (k === false) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a special case for when we select "No" and the select value is now false. If we pass false through the below $('<div/>').html(k).text() expression it will return an empty string which doesn't match any of the available <select> options which will make it look as if "No" is not selected. Passing true through $('<div/>').html(k).text(), however, will give you back a string "true" which makes the <select> work correctly.

@HeadyWook
Copy link

Awesome, thank you!

Copy link
Member

@Kukks Kukks left a comment

Choose a reason for hiding this comment

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

LGTM

@bolatovumar
Copy link
Contributor Author

The test that failed was for coin selection and is unrelated.

@NicolasDorier NicolasDorier merged commit 5cb647e into btcpayserver:master Feb 10, 2021
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.

Custom price display bug
5 participants