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

Filtering by multiple payment methods #1154

Closed
Renato1478 opened this issue Mar 4, 2024 · 6 comments
Closed

Filtering by multiple payment methods #1154

Renato1478 opened this issue Mar 4, 2024 · 6 comments
Assignees
Labels
enhancement 🆙 New feature or request

Comments

@Renato1478
Copy link
Contributor

Renato1478 commented Mar 4, 2024

Goal
As a Robosats user, I want to be able to filter by multiple payment methods.

Briefing
With the user willing to buy/sell in 2+ payment methods, sometimes he'll want to check the active orders in multiple payment methods instead of filtering the list all the time to check prices.

Additional context
In the "Create Order" functionality we can already select more than 1 payment method. Since the select field has the same design, I personally thought that I could do it in the order list as well.

image
"Multiple payment methods selection example"

@Renato1478
Copy link
Contributor Author

I'm willing to work on this feature if it's considered relevant to the app.

@Reckless-Satoshi Reckless-Satoshi added the enhancement 🆙 New feature or request label Mar 7, 2024
@Reckless-Satoshi
Copy link
Collaborator

Hey @Renato1478 . Indeed, this is a potentially useful feature.

The order book filtering only allows for one payment_method input at the moment. Several changes are needed as:

  • In desktop (large screens) the same Autocomplete field as seen in the Create page is used, however, it is limited to 1 entry.
  • On Mobile, it is a dropdown selection (so only 1 method allowed as well).

The filtering function, already accepts multiple payment methods:

const filterByPayment = function (order: PublicOrder, paymentMethods: any[]): boolean {
if (paymentMethods.length === 0) {
return true;
} else {
let result = false;
paymentMethods.forEach((method) => {
result = result || order.payment_method.includes(method.name);
});
return result;
}
};

I am not sure using the AutocompletePayment component on the Order book controls in mobile is a great idea, as selecting from a dropdown is way easier than writing on mobile. However, on desktop, it is likely very easy to fix the AutocomplePayment component on the book controls to accept several elements on the array of methods instead of just one.

If you want to work on it, we will gladly accept this PR. We can do a small tip from the devfund for the contribution of 20K Sats for this feature. Please, keep in mind our latest developments are in branch the-federation-layer-v0.6.0, so it would be best if you work on top of its tip commit :)

@Renato1478
Copy link
Contributor Author

Renato1478 commented Mar 8, 2024 via email

@Reckless-Satoshi
Copy link
Collaborator

Reckless-Satoshi commented Mar 9, 2024

Hey @Renato1478 , yes this could work for both mobile and desktop! The only drawback is the extra two clicks needed to open and confirm the new dialog. It's hard to say, do you think those two extra clicks are worthy in order to gain the edge ability to filter by more than one payment method at once?

I think it's a good case to bring up in the telegram group and see how users feel about it. It is definitely worth to implement if most users always look for several payment methods at once.

@Renato1478
Copy link
Contributor Author

Renato1478 commented Mar 12, 2024

Thanks for the reply! In respect on the extra clicks, I just opened the PR #1172 making this possible without creating extra clicks or the average user who wants to filter by just 1 method.

@Reckless-Satoshi
Copy link
Collaborator

#1172 was merged!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement 🆙 New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants