Details

Reviewers

emack

Group Reviewers

Restricted Project

Commits

rABCc32791f6ad4a: [Cashtab] Allow user to sort Agora offers by offer count

Summary

There are a number of complications to work through in optimizing how we present Agora offers to the user.

Cashtab has already gone though some iterations here.

Currently, each OrderBook component is well optimized to load all the info it needs to render. The Agora screen loads an OrderBook component for every `active offer.

Still, this is more than 500 components. And we do not have any way of sorting or searching these components.

This diff gives us a start. We initialize a map as a ref (so that it does not rerender when it changes). This map is passed to each OrderBook, so that each OrderBook can update it when it loads its own information.

When the map is loaded, the Agora screen is able to use this information to sort OrderBooks. For now, we only enable a sort by number of offers.

Going forward, we will enable sorting by price and depth.

After that, we can add more calculations to each OrderBook component to enable sorting by trading volume.

We still have more optimization work to do here. For example we should not render all 500 components on the screen, it causes the screen to go slowly and even the switch to perform this sort is slow.

But this is an important incremental improvement.

Test Plan

npm test

this is deployed at https://cashtab-local-dev.netlify.app/

navigate to agora screen. see the switch for sorting by offer count. It is disabled and a spinner is next to it. After the components finish loading, we can click this switch to sort by offer count.

It takes ~ 20 seconds for offers to load. Clicking the switch is itself a slow process. These things will be optimized later by not rendering all 500 + offers.

Diff Detail

Repository

rABC Bitcoin ABC

Branch

agora-get-search-data

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 31773
Build 63039: Build Diff	cashtab-tests
Build 63038: arc lint + arc unit

Event Timeline

bytesofman created this revision.Dec 26 2024, 19:50

Herald added a reviewer: Restricted Project. · View Herald TranscriptDec 26 2024, 19:50

Harbormaster completed remote builds in B31770: Diff 51754.Dec 26 2024, 19:55

remove debug log, better switch rendering

Harbormaster completed remote builds in B31771: Diff 51755.Dec 26 2024, 20:04

Build Bitcoin ABC Diffs / Diff Testing (cashtab-tests) failed.

Failed tests logs:

====== CashTab Unit Tests: <Agora /> We can see multiple offers, some we made, others we did not, and we can cancel an offer ======
Error: expect(element).not.toBeInTheDocument()

expected document not to contain element, found <div class="sc-cSHVUG jkArnh" title="Loading"><div /><div /><div /><div /></div> instead

Ignored nodes: comments, script, style
<html>
  <head />
  <body>
    <div>
      <div
        class="sc-hdPSEv dEfOCU"
      >
        <div
          class="Toastify"
        />
        <div
          class="sc-cmIlrE hrtokr"
        >
          <div
            class="sc-gleUXh lndbxp"
          >
            <div
              class="sc-hARARD cgmwYj"
            >
              <img
                alt="cashtab"
                class="sc-ccLTTT dharwH"
                src="test-file-stub"
              />
            </div>
            <div
              class="sc-bJHhxl fyvWUN"
            >
              <div
                class="sc-jvEmr eWcPZT"
              >
                <select
                  class="sc-hycgNl gBGlcN"
                  data-testid="wallet-select"
                  id="wallets"
                  name="wallets"
                >
                  <option
                    class="sc-chAAoq dRSlrf"
                    value="Agora Partial Beta"
                  >
                    Agora Partial Beta
                  </option>
                  <option
                    class="sc-chAAoq dRSlrf"
                    value="Agora Partial Alpha"
                  >
                    Agora Partial Alpha
                  </option>
                </select>
                <div
                  class="sc-fKGOjr dODwBM"
                >
                  <button
                    aria-label="Copy ecash:qreq3mm4avxaw782g4qvhktx4qcv0w2tkqj3j5jaad"
                    class="sc-frDJqD iDiUEE"
                  >
                    <svg
                      title="copy-paste"
                    />
                  </button>
                  <div
                    class="sc-dymIpo hWxICM"
                  >
                    <div
                      class="sc-bnXvFD dzwPxp"
                    >
                      <input
                        checked=""
                        class="sc-gJWqzi efBCgh"
                        id="show-hide-balance"
                        name="show-hide-balance"
                        title="show-hide-balance"
                        type="checkbox"
                      />
                      <label
                        class="sc-gFaPwZ ghvKrk"
                        for="show-hide-balance"
                      >
                        <span
                          class="sc-fhYwyz cZDTKh"
                          data-off=""
                          data-on=""
                        />
                        <span
                          class="sc-jzgbtB dPhrEl"
                        />
                      </label>
                    </div>
                  </div>
                </div>
              </div>
              <div
                class="sc-TuwoP jRfJNF"
                title="Wallet Info"
              >
                <div
                  class="sc-dTLGrV kpTbFj"
                  title="Balance in XEC"
                >
                  42.00
                   
                  XEC
                   
                </div>
                <div
                  class="sc-ivVeuv htUdFg"
                  title="Balance in Local Currency"
                >
                  $
                  0.00
                   
                  USD
                </div>
                <p
                  class="sc-cCbXAZ hmwpIQ"
                  title="Price in Local Currency"
                >
                  1 
                  XEC
                   = 
                  0.00003000
                   
                  USD
                </p>
              </div>
            </div>
            <div
              class="sc-eNPDpu iGZqUQ"
            >
              <h2
                class="sc-kafWEX eIsEHa"
              >
                Agora 
                <svg
                  title="Meme Agora"
                />
              </h2>
              <div
                class="sc-ciodno ekemxG"
                title="Active Offers"
              >
                <div
                  class="sc-jKVCRD gNurmH"
                >
                  <div
                    class="sc-dymIpo hWxICM"
                  >
                    <div
                      class="sc-bnXvFD fHgwZD"
                    >
                      <input
                        class="sc-gJWqzi efBCgh"
                        id="Toggle Active Offers"
                        name="Toggle Active Offers"
                        title="Toggle Active Offers"
                        type="checkbox"
                      />
                      <label
                        class="sc-gFaPwZ ghvKrk"
                        for="Toggle Active Offers"
                      >
                        <span
                          class="sc-fhY...
    at toBeInTheDocument (/work/cashtab/src/components/Agora/__tests__/index.test.js:525:56)
    at runWithExpensiveErrorDiagnosticsDisabled (/work/cashtab/node_modules/@testing-library/dom/dist/config.js:47:12)
    at checkCallback (/work/cashtab/node_modules/@testing-library/dom/dist/wait-for.js:124:77)
    at checkRealTimersCallback (/work/cashtab/node_modules/@testing-library/dom/dist/wait-for.js:118:16)
    at Timeout.task [as _onTimeout] (/work/cashtab/node_modules/jsdom/lib/jsdom/browser/Window.js:520:19)
    at listOnTimeout (node:internal/timers:581:17)
    at processTimers (node:internal/timers:519:7)

Each failure log is accessible here:
CashTab Unit Tests: <Agora /> We can see multiple offers, some we made, others we did not, and we can cancel an offer

confirm with a test

bytesofman edited the summary of this revision. (Show Details)Dec 26 2024, 21:53

bytesofman edited the test plan for this revision. (Show Details)

Harbormaster completed remote builds in B31773: Diff 51758.Dec 26 2024, 21:54

support sorting by tokenid

Harbormaster completed remote builds in B31774: Diff 51759.Dec 26 2024, 22:14

show the switch disabled with a spinner until it is ready to roll

Harbormaster completed remote builds in B31775: Diff 51760.Dec 26 2024, 22:19

bytesofman published this revision for review.Dec 26 2024, 22:26

bytesofman edited the test plan for this revision. (Show Details)

Do you know where you want to go with this change ? I mean the end filters.

This choice of "sort by number of offers" is a bit weird too me. I would expect something like "sort by trending first" (e.g. the number of offers over a 7 days period), "sort by most popular" (e.g. the highest amount of sell value), etc.

If anything I would change the name with something less techy

In D17402#395972, @Fabien wrote:

Do you know where you want to go with this change ? I mean the end filters.

Not exactly, though here are some I would like to have for sure

by trading volume
by custom whitelist
by depth

This choice of "sort by number of offers" is a bit weird too me. I would expect something like "sort by trending first" (e.g. the number of offers over a 7 days period), "sort by most popular" (e.g. the highest amount of sell value), etc.

Yes, the main thing I am doing in this diff is solving the problem of "how to get info about all of the offers from distinct components that each load info only about themselves." Sorting by offer here is really just showing that this approach can work; it's the simplest available sort and seems to be a (rough) proxy for volume until I can add that.

If anything I would change the name with something less techy

++ yes, there needs to be some more thought in terms of what categories are sorted. This is mostly a diff for "here is how we can get all the info." Going forward, we can change the shape of the OrderBookInfo interface and incrementally add more user-facing options.

emack requested changes to this revision.Dec 29 2024, 00:01

emack added a subscriber: emack.

emack added inline comments.

cashtab/src/components/Agora/index.tsx
323 ↗	(On Diff #51760)	Setting up an interval just for one button's clickable state seems a lot of work. Can the `orderBookInfoMapRef.current.size === activeOffersCashtab.offeredFungibleTokenIds.length` check be at the end of each iteration that loads each orderbook? And then updates `setAllOrderBooksLoaded` so the rest of this diff remains unchanged.

This revision now requires changes to proceed.Dec 29 2024, 00:01

Also the first toggle's functionality being different to the bottom 2 should be more obvious, like using Tabs for Buy/Manage and toggles for sorting. Or simply a dropdown for sorting so you don't end up with 5 toggles for 5 sorting options.

In D17402#396020, @emack wrote:

Also the first toggle's functionality being different to the bottom 2 should be more obvious, like using Tabs for Buy/Manage and toggles for sorting. Or simply a dropdown for sorting so you don't end up with 5 toggles for 5 sorting options.

I think the solution here needs to be outside of this diff for a couple of reasons.

The whole "Toggle Buy/Manage listings" logic is bad. I think we need a better component to organize and present all listings created by the user, rather than shoehorning this into screens designed to be storefronts. So, this should be pulled out...but it needs to be its own refactor.
Switches are a simple way to filter a large list, esp on mobile. Going forward, need more thought in what options should be available. But the Switch UX is the easiest. I like how it turned out on the Etokens screen. There is prob a better way to handle this, but it is a design problem that I don't think can be worked out before we know all the sorting dimensions.

cashtab/src/components/Agora/index.tsx
323 ↗	(On Diff #51760)	There is some complexity here, to be sure. However it's the simplest solution I have found so far. Alternatives Instead of having each OrderBook load and present its own offer info, get it all in the parent Agora component. This "works" but it takes much longer, even with Promise.all(). Tried several variations of this. All ends up slower than what we have now, which is already pretty slow. Instead of a ref, make the map a state variable and `useEffect` check as it changes. I was not able to get this to work. The issue is you have to `setState` for `useEffect` to know that something has changed. Having 100s (thousands soon) of OrderBook components all calling `setState` on a map -- triggerring re-renders for every call -- imo much worse than the interval. We are ultimately here waiting to fetch data from an indexer. The process takes time. So, a time-based interval is an appropriate solution. Going forward -- I do not at all think this is the optimal solution. We will need to figure out ways to limit how much info we are really asking for from the server. But I don't think we can optimize in this way until we have some kind of whitelisting system. Even then, the user should have some option to see all the offers, even if it is a lengthy process. Even with limited info, though, a ref that we check with `useInterval` is probably the best way to handle the general problem. It's the only way I could find to do it without setting state and without duplicating the already heavy API calls.

emack accepted this revision.Dec 29 2024, 07:38

This revision is now accepted and ready to land.Dec 29 2024, 07:38

Closed by commit rABCc32791f6ad4a: [Cashtab] Allow user to sort Agora offers by offer count (authored by bytesofman). · Explain WhyDec 29 2024, 11:42

This revision was automatically updated to reflect the committed changes.

bytesofman marked an inline comment as done.

bytesofman added a commit: rABCc32791f6ad4a: [Cashtab] Allow user to sort Agora offers by offer count.

[Cashtab] Allow user to sort Agora offers by offer count
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 51758

cashtab/src/components/Agora/OrderBook/index.tsx

cashtab/src/components/Agora/tests/index.test.js

cashtab/src/components/Agora/index.tsx

cashtab/src/components/Common/Spinner.tsx

[Cashtab] Allow user to sort Agora offers by offer countClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 51758

cashtab/src/components/Agora/OrderBook/index.tsx

cashtab/src/components/Agora/__tests__/index.test.js

cashtab/src/components/Agora/index.tsx

cashtab/src/components/Common/Spinner.tsx

[Cashtab] Allow user to sort Agora offers by offer count
ClosedPublic
Actions

Revision Contents
Changeset List

cashtab/src/components/Agora/tests/index.test.js