Explorer Long Query bountie - add loader on inventory tab to give feedback to user

[jotagep]

A simple UX solution to give a feedback to user that inventory assets fetch is running on background.

[agileurbanite]

[agileurbanite]

[agileurbanite]

is it also appropriate to cache some of these requests on the client side? Every time inventory is clicked it re-fetches, not sure if that's normal.

[y04nqt]

Yeah, we could definitely improve the way we handle the fetching here. That would be a good requirement to institute now than later since we are in the area.

I'll also dig into this project a bit further myself when I have time. I haven't dug into much of the code just yet.

I think another good thing to do with these findings is to make some additional bounties from things we find now. We should keep this scope narrow to just addressing the lockup fix and user feedback experience.

[jotagep]

@y04nqt @agileurbanite the locked up and runtime problems that you attached, are caused when clicking on Holders tab, not related to inventory. In inventory tab we just fetch data one time.

[jotagep]

@y04nqt Yes, sorry, you can check the functionality in two ways

• You can introduce an erc721 in search bar, for example (0xd7745a515bedd91ee9024b6c31dc1e75a10dc618)
• Then we can see, Inventory tab disabled with a beauty spinner until we receive all inventory assets.

Also you can check

• You can insert url directly (address/0xd7745a515bedd91ee9024b6c31dc1e75a10dc618?activeTab=6) with inventory tab active.

• Then we can see, same than before on inventory tab and a box with a loader spinner to keep coherence with another pages like Transactions, until we receive assets response from api.

  I'm gonna try to solve the holders tab lock up and i add it to this PR, ok?

[agileurbanite]

I agree on creating separate bounties, I'll drop from this one but happy to help @jotagep with the review. @y04nqt we should consider triggering a GH action to test the PR to determine if it's safe to merge. For example, did code coverage drop, are there corresponding tests in place etc.

[jotagep]

@y04nqt I added a new commit. i've decided run mapping function of inventory assets from a web worker to avoid blocks in the UI. Also i add a hook use-deep-compare-effects by Kent.C.Dodds to avoid rerender on holders tab

• lock-up ui problems was solved 👌

[agileurbanite]

@jotagep you don't think adding use-deep-compare-effects is a stop gap? I think there's some opportunities here that deal with improving reconciliation. By the way, this comment is not a blocker for merging this PR.

[agileurbanite]

just tested this locally and LGTM 🚢 🚢

[jotagep]

@agileurbanite i just added use-deep-compare-effects to avoid rerender when change filters on holders tab, i don't think is a stop gap. The real problem was here, is the way of mapping a big bunch of data that overload the main thread. For this reason a web worker is a good option to not block UI.

On the other hand, it is clear that the way of fetching data and cache in whole repo is not the most efficient. Maybe we have to take a look in the future.

Thanks for your comments and reviews @agileurbanite 😊

[y04nqt]

I'll give this another review today. I'll also figure out the bounty reward process as well and update once this is clear! :)

[jotagep]

Perfect ✌️

[jotagep]

@y04nqt Great! feel free to merge,

[roger-maddocks]

Is this bounty considered closed after jotagep's additions?

[jotagep]

@roger-maddocks Ask to @y04nqt , he is managing this bounty

[jotagep]

@y04nqt how is this bountie??