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

Update currency filtering logic #6041

Merged
merged 2 commits into from Mar 9, 2022
Merged

Update currency filtering logic #6041

merged 2 commits into from Mar 9, 2022

Conversation

ghost
Copy link

@ghost ghost commented Feb 12, 2022

Fixes #5300

@ghost ghost mentioned this pull request Feb 15, 2022
@alkum
Copy link
Contributor

alkum commented Feb 17, 2022

ACK.

All currencies are enabled by default only for new installations, but that's fine.

@ripcurlx ripcurlx added this to the v1.8.4 milestone Feb 17, 2022
@ripcurlx
Copy link
Contributor

For this change I'd like to get more consensus as it will show for the initial dropdown a list with only one market that has actual offers. (AUD has some Mainnet offers)
Bildschirmfoto 2022-02-28 um 13 09 41

I agree that this pre-selection is not such a big thing anymore since we have the search possibility in the combobox we haven't had in the past. Personally I'd be fine with this change if we also change the sorting in the comboxbox from sorting by name to the currency code (first column value) which is more intuitive IMO.

@chimp1984 @m52go @UX-P @leo816

@m52go
Copy link
Contributor

m52go commented Feb 28, 2022

Is there any way to sort currencies by trading activity? Maybe not real-time but even updated manually would work since the overall trends don't change very much.

@pazza83
Copy link

pazza83 commented Mar 1, 2022

Is there any way to sort currencies by trading activity? Maybe not real-time but even updated manually would work since the overall trends don't change very much.

It is done in a similar way as proposed; number of current offers, in Market > Offer Book

This seems to work well. Could this be implemented for users going via Buy / Sell ?

Market - offer book

@m52go
Copy link
Contributor

m52go commented Mar 1, 2022

Interesting. Mine appear to be sorted alphabetically in spite of having the sort-by-number-of-offers option ticked on in settings. But I do remember my drop-down looking like yours at some point in the past...so I'm not sure what's going on.

2022-02-28_22-24

@pazza83
Copy link

pazza83 commented Mar 1, 2022

@m52go

I think the option is in Setting > Preferences

Sort market lists with no. of offers / trades

@m52go
Copy link
Contributor

m52go commented Mar 1, 2022

Yes, as I said, that option is ticked on but not working for some reason...seems like a bug.

@chimp1984
Copy link
Contributor

NACK

I think to spam the whole list with a multitude of currencies never traded is decreasing usability.
The related issue also do not suggest that solution but only complains inconsistency.

I think a better solution would be to show main currencies as we have, but use for the filter predicate the whole list of all currencies so that If I search for a currency which is not in the default list (or user defined list) it still is found.

@ghost
Copy link
Author

ghost commented Mar 3, 2022

The related issue also do not suggest that solution but only complains inconsistency.

That solution was suggested by @pazza83 in discussion on issue, however I also wasn't sure if it's the best one.

I think a better solution would be to show main currencies as we have, but use for the filter predicate the whole list of all currencies so that If I search for a currency which is not in the default list (or user defined list) it still is found.

I like it, I'll check how difficult it is to implement it that way.

@ghost ghost changed the title Make all currencies enabled by default Update currency filtering logic Mar 3, 2022
@ghost
Copy link
Author

ghost commented Mar 3, 2022

I pushed solution suggested by @chimp1984 :

  • when user type single letter default/user defined list is used for search
  • when user type two or more letters list of all currencies is used for search

leo816
leo816 previously approved these changes Mar 7, 2022
@chimp1984
Copy link
Contributor

I pushed solution suggested by @chimp1984 :

* when user type single letter default/user defined list is used for search

* when user type two or more letters list of all currencies is used for search

I think that is confusing.
Just show the user defined list when opening the dropdown and apply the search to all currencies.

@chimp1984
Copy link
Contributor

NACK

@ghost
Copy link
Author

ghost commented Mar 8, 2022

Just show the user defined list when opening the dropdown and apply the search to all currencies.

Fixed

Copy link
Contributor

@ripcurlx ripcurlx left a comment

Choose a reason for hiding this comment

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

ACK - Tested it on Mainnet

All suggested changes in comments have been implemented.

@ripcurlx ripcurlx merged commit 43fb646 into bisq-network:master Mar 9, 2022
@ghost ghost mentioned this pull request Mar 25, 2022
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.

Currency filtering is inconsistent
6 participants