kodadot / nft-gallery

Generative Art Marketplace
https://koda.art
MIT License
638 stars 360 forks source link

follow up on Collection Activity Tab // limit number of events in query #5388

Open daiagi opened 1 year ago

daiagi commented 1 year ago

to sum it up, left overs for next PR:

exezbcz commented 1 year ago

scroll position reset when switching tabs (items/activity)

from the bottom of this comment: https://github.com/kodadot/nft-gallery/pull/5345#issuecomment-1484042730

  • can we make it so the switch between items and activity does not reset the scroll position
    • it's better when its seamless
daiagi commented 1 year ago

by @roiLeo : https://github.com/kodadot/nft-gallery/pull/5345#pullrequestreview-1358436762

yangwao commented 1 year ago

Plus codeclimate review

daiagi commented 1 year ago

@roiLeo About the point you've raised that there is no limit on events I think it is necessary to get all events in order to have correct flippers info I guess a way to reduce data is to get all buy events And paginate the rest as user scrolls down the list

But I don't that will make much difference in query size

Wdyt?

roiLeo commented 1 year ago

@roiLeo About the point you've raised that there is no limit on events I think it is necessary to get all events in order to have correct flippers info I guess a way to reduce data is to get all buy events And paginate the rest as user scrolls down the list

Yes! Limit 20-50 and paginate the rest. (we had same stuff in https://github.com/kodadot/nft-gallery/pull/5038)

But I don't that will make much difference in query size

Wdyt?

Let's imagine I make 100k+ "LIST" event, client side won't be able to handle all this data.

daiagi commented 1 year ago

OK. Just to clarify... You do agree that all buy events will still be needed to be fetched immediately, right?

Let's imagine I make 100k+ "LIST" event, client side won't be able to handle all this data.

Yes. Of course

@vikiival what is your take on this? What would be most efficient way?

roiLeo commented 1 year ago

OK. Just to clarify... You do agree that all buy events will still be needed to be fetched immediately, right?

for me not necessarily. Is it for the graph? this would be the events over a given period of time.

daiagi commented 1 year ago

The graph too I guess But I was thinking about flippers tab, for that I need to know about every time an nft switch hands, so that means all buy events

@vikiival would it make sense to process holders and flippers on rubick and add these attributes to collection Entity? Especially flippers

By the way, on previous collection activity minters who sold their nft are also dubbed flippers, I disagree with that. I think only someone who bought and sold is a flipper. For now I kept this as it was before though

daiagi commented 1 year ago

by @roiLeo : #5345 (review)

* I would remove `import { set } from 'vue'`

* make `has-text-k-blue` & `has-text-k-grey` global [variables](https://bulma.io/documentation/customize/variables/) class     

* put composable (`useReplaceUrl`) in composable folder

* relative path

* duplicate css code & some bad selector (`.background-color`??)

all done.

outstanding issues:

daiagi commented 1 year ago

scroll position reset when switching tabs (items/activity)

Fixed by #5416

daiagi commented 1 year ago

@vikiival @roiLeo I think creating a userEntity is the way forward A userEntity will have all the data required for holders flippers tab and will take the load of calculating it off the front end. Does that make sense?

Thank you

roiLeo commented 1 year ago

@vikiival @roiLeo I think creating a userEntity is the way forward A userEntity will have all the data required for holders flippers tab and will take the load of calculating it off the front end. Does that make sense?

Yes! kodadot/rubick#52 & kodadot/rubick#56 (maybe kodadot/rubick#157 for a start) Feel free to try to write new schema, I can help you if you need to setup indexer project.

daiagi commented 1 year ago

it will take some time as I'm in vacation until 9th of April

If you are interested in getting it going feel free to, otherwise I will try my strength in it when I'm back

vikiival commented 1 year ago

9th of April

Will take that time to design new Schema 2.0 😅

yangwao commented 1 year ago

Yes! https://github.com/kodadot/rubick/issues/52 & https://github.com/kodadot/rubick/issues/56 (maybe https://github.com/kodadot/rubick/pull/157 for a start) limit number of events in query, paginate events table

Only this is left in this issue?

roiLeo commented 1 year ago

limit number of events in query, paginate events table

Only this is left in this issue?

Yes imo same stuff as #5540

yangwao commented 1 year ago

this is related I guess

Good for resolver?

yangwao commented 1 year ago

hey @daiagi can we look on this as we want to launch rmrk2 bug hunting

daiagi commented 1 year ago

hey @yangwao it is waiting on this:

once we can fetch holder/flippers info as part of new userEntity, the need to fetch all events on FE in order to calculate them will be eliminated. then we can paginate the events table in activity tab

That should resolve all heavy loading isuues in activity tab

yangwao commented 1 year ago

it is waiting on this:

and would be possible make lazy loading for that scroll table for now? I think that would save day till then? Because it's last thing we want to launch bug hunting for rmrk2 and it's quite issue for collection where are lot of owners, like >100.

daiagi commented 1 year ago

events table is already lazy loaded I've lazy loaded holders/flippers tabs in #5608 and seeing performance improvement

JustLuuuu commented 2 months ago

@roiLeo can we close this?

roiLeo commented 2 months ago

@roiLeo can we close this?

Im not aware of current implementation, it was needed to limit number of events in query so user won't face potential bug where there is more than 1k events.

Feel free to close it. If a bug occur on a big collection it could be related to that one

vikiival commented 2 months ago

Still an issue but we would need resolver