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

A limit of 20 CryptoKitties is displayed in the collectibles list in the wallet with incorrect order #6839

Closed
SlizkovaGanna opened this issue Nov 22, 2018 · 23 comments · Fixed by #9968

Comments

@SlizkovaGanna
Copy link

SlizkovaGanna commented Nov 22, 2018

Type: Bug

Summary: As a user, I would like to see the correct number of my collectibles in the wallet, in the right order.
Currently, only 20 CK are shown, without any option to show more of them.
As a user, I would be very confused

Expected behavior

The exact number of collectibles should be displayed in the correct order

Actual behavior

Only 20 CK are displayed in a different order from the App

Reproduction

  1. Recover/open an account that has more than 20 collectibles.
  2. Navigate to the wallet and click '>' button next to CryptoKitties collectibles in the list.

image

  1. List through them to make sure that the valid number of collectibles is displayed
  2. Go to the CK App to check that the order that collectibles are displayed is the same

Collectibles list, top of the list:

image

App list, top of the list:

image

Additional Information

Status version: 0.9.31 (1811211215); node 27700aa2 (Nov 22 nightly build)
Operating System: iOS, Android

@SlizkovaGanna
Copy link
Author

Was not sure if this is a bug or a feature request, but according to the description in #5017 - it's a bug.
@flexsurfer your feedback is appreciated

@flexsurfer
Copy link
Member

it's a bug for sure, thank you

@asemiankevich
Copy link
Contributor

@hetvart please add a test for this when you have time. This is definitely a regression

@hetvart
Copy link
Contributor

hetvart commented Nov 22, 2018

I set it as the top priority task!

@hetvart
Copy link
Contributor

hetvart commented Nov 23, 2018

Postponed it. Probably till I get an account with the needed amount of test collectibles.

@flexsurfer
Copy link
Member

@asemiankevich did you test more than 20 CKs ? and it worked?

@asemiankevich
Copy link
Contributor

i dont remember exactly how many kitties i did have once testing #5017. However now i have 23 kitties and only 20 is shown.

@flexsurfer
Copy link
Member

i mean i'm not sure it's regression, i remember i didn't have more than 20 CKs, so i couldn't test properly

@asemiankevich
Copy link
Contributor

wrong order is a regression anyway

@ghost
Copy link

ghost commented Feb 21, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@ghost ghost added the stale label Feb 21, 2019
@ghost
Copy link

ghost commented Feb 28, 2019

This issue has been automatically closed. Please re-open if this issue is important to you.

@ghost ghost closed this as completed Feb 28, 2019
@asemiankevich asemiankevich reopened this Aug 29, 2019
@asemiankevich
Copy link
Contributor

@rachelhamlin can we consider this?

@rachelhamlin
Copy link
Contributor

Adding to nice-to-have list, not blocker :)

@gitcoinbot
Copy link

Issue Status: 1. Open 2. Started 3. Submitted 4. Done


This issue now has a funding of 90.0 DAI (90.0 USD @ $1.0/DAI) attached to it.

@gitcoinbot
Copy link

gitcoinbot commented Jan 6, 2020

Issue Status: 1. Open 2. Started 3. Submitted 4. Done


Work has been started.

These users each claimed they can complete the work by 2 weeks, 6 days from now.
Please review their action plans below:

1) samgos has started work.

React is my bread to my butter and I'm able to read through other's code pretty well, let's give it a shot, will be done ASAP.

Learn more on the Gitcoin Issue Details page.

@StatusSceptre
Copy link
Member

Welcome aboard @SamGos - you're doing to need to write Clojure and compile it to RN. Any Clojurescript familiarity? Here's our quickstart, just ping us with any questions, we're happy to help.

@deomaius
Copy link

deomaius commented Jan 7, 2020

Thank you for the approval, I will not let you down, Clojure no but the JVM yes. I'm sure I'll be fine, will do.

@deomaius
Copy link

Got the all the kitties showing, so now it's just a matter indexing them in the correct order, nearly there!

@StatusSceptre
Copy link
Member

Wow, awesome work @SamGos! Thanks for the update.

@deomaius
Copy link

deomaius commented Jan 13, 2020

I'm afraid I cannot find a viable solution for indexing the kitties correctly due to Clojure's abnormal behaviour when it comes to mapping and indexing values. Alongside working with http-get-n (which I can't find out for the life of me what library extends from this) and it's exclusivity to use mapv, I did find that filterv worked perfectly for maintaining the order of the array but was faced with:

TypeError: cb is not a function. (In 'cb(value)', 'cb' is "TypeError: Network request failed")

I have spent a surplus of 40 hours on this task, so it is no longer financially viable for me to continue. I'm sure you're developers could easily fix the sorting in a few minutes or perhaps give me some guidance, I would be more than happy to finish the job if so. I am not adept at Clojure and this is my first time using the language. Regardless I fixed the major problem of this issue I am just wondering am I still compliant for the reward.

Repo: https://github.com/samgos/status-react

@deomaius
Copy link

Opened a WIP PR at #9810, hopefully it may shine some light on how to implement the indexing.

@StatusSceptre
Copy link
Member

Thanks for the great effort @SamGos! I'll definitely compensate you for your time.

@deomaius
Copy link

That's awesome I'll submit the work on gitcoin then, forgive me for not sorting the indexing but at least the kitties are showing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
8 participants