Add total sold vs available widget #2898

chinins · 2021-11-16T20:29:11Z

Description

This PR adds total sold vs. available widget and adds some updates to tokens display in the remaining widget.

New stuff ✨

add new widget & corresponding queries
add react-textfit package
add updated numbers formatting

TODO

the numbers in sold vs. available widgets should go into 2 lines when they hit 10px - this is now done! :)

Resolves #2868

ArmandoGraterol

Nice! I like how it looks and the new info is quite useful when testing the coin machine. Surely it's also gonna be useful for normal users.

When I enter the coin machine I'm seeing some warnings on the console:

Can you check them out?

arrenv · 2021-11-24T18:40:27Z

Looks like the perfectFit property was dropped - See reference: https://github.com/malte-wessel/react-textfit/blob/fc7aa18e61a2397fcef838dc8b16c8dc027dce26/CHANGELOG.md#v100

arrenv

Looks to be working well.

chinins · 2021-11-25T13:20:16Z

@ArmandoGraterol, it's a leftover prop I forgot to remove - thank you for spotting it! It should be all good now - without any warning.

arrenv

Working well, without the error now for me!

ArmandoGraterol

Yep, from what I've tested it's working as expected and without warnings! Good job

chinins · 2021-11-25T18:11:49Z

@ArmandoGraterol , @arrenv , can you please review my latest commit and test again?

I have added the dynamic switching to 2 lines when the font-size reaches 10px.

arrenv

Nice one, the split line looks to be working well.

ArmandoGraterol

The multiline seems to be working as expected!

rdig

Congrats on putting this PR together. Very nice work! 💯

There still is a issue in here. (but which was inherited from the previous iteration)

If the coin machine has less tokens that the batch available, it will still show the batch as having available the "max" allowed, even though the coin machine doesn't have them anymore.

I'll show you some pictures since it's easier to explain.

In here, there are only 10 tokens available in the coin machine (which the "Total sold v. available correctly shows -- 70/80), the buy widget correctly only allows you to buy 10 (notice the error), but the "Batch sold v. available" widget still shows 0 / 40 which is incorrect in this case as there are only 10 tokens left. It should show 0 / 10.

Notice, that if I pass the period, the previous batches table shows the period, correctly, as having just 0 / 10

rdig · 2021-11-25T23:28:11Z

src/data/resolvers/coinMachine.ts

+          tokenClient.filters.Transfer(null, coinMachineClient.address, null),
+        );


This is very expensive, but I guess it's what we have to do in order to fetch the token transfers into the coin machine.

However, you can filter it even more, by only fetching blocks from the time the current instance of the coin machine was installed.

See this for an example:

colonyDapp/src/data/resolvers/coinMachine.ts

Lines 329 to 356 in 9ba1bbf

const subgraphData = await apolloClient.query<

SubgraphCoinMachinePeriodsQuery,

SubgraphCoinMachinePeriodsQueryVariables

>({

query: SubgraphCoinMachinePeriodsDocument,

variables: {

colonyAddress: colonyAddress.toLowerCase(),

extensionAddress: coinMachineClient.address.toLowerCase(),

limit,

},

fetchPolicy: 'network-only',

});

const [extensionInitialised] = (

subgraphData?.data?.extensionInitialisedEvents || []

).map(parseSubgraphEvent);

/*

* We use the `FromChain` suffix to make these events more easily

* recognizable when reading the code

*/

const transferLogsFromChain = await getLogs(

tokenClient,

tokenClient.filters.Transfer(null, coinMachineClient.address, null),

{

fromBlock: extensionInitialised?.blockNumber,

},

);

Fair enough - done now.

rdig · 2021-11-25T23:29:41Z

src/modules/dashboard/components/CoinMachine/BuyTokens/BuyTokens.tsx

@@ -201,7 +201,7 @@ const BuyTokens = ({
  const getFormattedCost = useCallback(
    (amount) => {
      const decimalCost = new Decimal(amount)
-        .times(salePriceData.coinMachineCurrentPeriodPrice)
+        .times(salePriceData?.coinMachineCurrentPeriodPrice || '0')


Shouldn't this actually be OR 1, because if you multiply by 0, you'll always get 0, but if you multiply by 1 you'll just get the amount.

Depends on what you want the fallback to be.

I think the default value should be 0. If it's 0 it will give an indication that something is off more that if it defaults to 1.... But I am happy to hear a counter-argument

rdig · 2021-11-25T23:33:00Z

src/modules/dashboard/components/CoinMachine/CoinMachine.tsx

@@ -311,6 +346,7 @@ const CoinMachine = ({
            }}
            sellableToken={sellableToken}
            purchaseToken={purchaseToken}
+            periodTokens={periodTokens}


I really that you put the period tokens back in here, as I worked really hard to refactor this so that it expressly doesn't use this specific value as it's not quite optimized.

However, I do understand why it's here, because you had to move the "Price next batch" display into the table.

I really don't like this, but this is a side-effect of just refactoring design willy nilly, you end up with content you need to move around, and you just slap it somewhere....

(sorry for the rant :) )

I am not sure I understand why it's so bad as I just moved the Price next batch logic from one component to another and added one prop to TokenSalesTable...
I am happy to discuss this & refactor once I understand the ideal state you would like to see.

chinins

@rdig updated the PR. Not sure about your comment regarding the periodTokens prop passing, other than that it's ready for re-review.

chinins · 2021-11-26T17:36:53Z

src/modules/dashboard/components/CoinMachine/CoinMachine.tsx

@@ -311,6 +346,7 @@ const CoinMachine = ({
            }}
            sellableToken={sellableToken}
            purchaseToken={purchaseToken}
+            periodTokens={periodTokens}


I am not sure I understand why it's so bad as I just moved the Price next batch logic from one component to another and added one prop to TokenSalesTable...
I am happy to discuss this & refactor once I understand the ideal state you would like to see.

chinins · 2021-11-26T17:37:50Z

src/data/resolvers/coinMachine.ts

+          tokenClient.filters.Transfer(null, coinMachineClient.address, null),
+        );


Fair enough - done now.

chinins · 2021-11-26T17:39:46Z

src/modules/dashboard/components/CoinMachine/BuyTokens/BuyTokens.tsx

@@ -201,7 +201,7 @@ const BuyTokens = ({
  const getFormattedCost = useCallback(
    (amount) => {
      const decimalCost = new Decimal(amount)
-        .times(salePriceData.coinMachineCurrentPeriodPrice)
+        .times(salePriceData?.coinMachineCurrentPeriodPrice || '0')


I think the default value should be 0. If it's 0 it will give an indication that something is off more that if it defaults to 1.... But I am happy to hear a counter-argument

rdig

Working as expected now. Very nice turnaround. 🥇

On Monday I'll put it up on QA

chinins added the feature label Nov 16, 2021

chinins self-assigned this Nov 16, 2021

chinins marked this pull request as draft November 16, 2021 20:29

chinins changed the base branch from master to feature/coin-machine-redux November 16, 2021 20:29

rdig force-pushed the feature/coin-machine-redux branch from 9e79a65 to 9cc7853 Compare November 17, 2021 18:46

chinins force-pushed the feature/2868-add-total-sold-vs-available-widget branch from 6d1c360 to b5d4612 Compare November 18, 2021 11:38

Base automatically changed from feature/coin-machine-redux to master November 18, 2021 12:52

chinins force-pushed the feature/2868-add-total-sold-vs-available-widget branch from 444e848 to c1a32c3 Compare November 22, 2021 12:11

chinins requested a review from a team November 23, 2021 13:14

chinins marked this pull request as ready for review November 23, 2021 13:14

chinins marked this pull request as draft November 23, 2021 13:21

chinins marked this pull request as ready for review November 23, 2021 15:29

chinins force-pushed the feature/2868-add-total-sold-vs-available-widget branch from 85fa442 to dec86a0 Compare November 23, 2021 15:33

ArmandoGraterol reviewed Nov 23, 2021

View reviewed changes

arrenv reviewed Nov 24, 2021

View reviewed changes

arrenv approved these changes Nov 25, 2021

View reviewed changes

chinins force-pushed the feature/2868-add-total-sold-vs-available-widget branch from 7f453ac to ec2406e Compare November 25, 2021 15:09

ArmandoGraterol approved these changes Nov 25, 2021

View reviewed changes

chinins force-pushed the feature/2868-add-total-sold-vs-available-widget branch from f089125 to 9ba1bbf Compare November 25, 2021 18:29

arrenv approved these changes Nov 25, 2021

View reviewed changes

ArmandoGraterol approved these changes Nov 25, 2021

View reviewed changes

rdig requested changes Nov 25, 2021

View reviewed changes

chinins commented Nov 26, 2021

View reviewed changes

chinins added 4 commits November 26, 2021 18:42

Feat: reorganise CoinMachine & add total tokens widget

5e84c9e

Feat: add total tokens query, types, resolver & calculations to dapp

dc0dafb

Chore: change subgraph back to the correct commit

61cc05a

Feat: add proper styles to the next batch price to the TokenSalesTable

48bce19

chinins added 10 commits November 26, 2021 18:42

Refactor: fixed missing props in CoinMachine caused by rebase

0065b8d

Feat: add dynamic text resizing to RemainingDisplayWidget

e8e5e44

Feat: add dynamic decimal calc & number formatting to remaining tokens

d13b3d0

Refactor: change FormattedNumber to Numeral in remaining tokens

7eedbf0

Refactor: fix a little bug in BuyTokens when data not yet available

cb8a27c

Refactor: remove font-size coming from Numberal in remaining display

41d47f5

Refactor: remove old leftover prop for the Textfit component

ef1de5d

Feat: add dynamic switch to 2 lines if font-size reaches 10px

25a6c9b

Fix: show available left tokens if it is less than target per period

f01f22b

Feat: add fromBlock fliter in coinMachineTotalTokens resolver

272a49e

chinins force-pushed the feature/2868-add-total-sold-vs-available-widget branch from b807101 to 272a49e Compare November 26, 2021 17:55

rdig approved these changes Nov 26, 2021

View reviewed changes

chinins merged commit 4c8c017 into master Nov 27, 2021

chinins deleted the feature/2868-add-total-sold-vs-available-widget branch November 27, 2021 15:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add total sold vs available widget #2898

Add total sold vs available widget #2898

chinins commented Nov 16, 2021 •

edited

Loading

ArmandoGraterol left a comment

arrenv commented Nov 24, 2021

arrenv left a comment

chinins commented Nov 25, 2021

arrenv left a comment

ArmandoGraterol left a comment

chinins commented Nov 25, 2021

arrenv left a comment

ArmandoGraterol left a comment

rdig left a comment

rdig Nov 25, 2021

chinins Nov 26, 2021

rdig Nov 25, 2021

chinins Nov 26, 2021

rdig Nov 25, 2021

chinins Nov 26, 2021

chinins left a comment

chinins Nov 26, 2021

chinins Nov 26, 2021

chinins Nov 26, 2021

rdig left a comment

		tokenClient.filters.Transfer(null, coinMachineClient.address, null),
		);

	const subgraphData = await apolloClient.query<
	SubgraphCoinMachinePeriodsQuery,
	SubgraphCoinMachinePeriodsQueryVariables
	>({
	query: SubgraphCoinMachinePeriodsDocument,
	variables: {
	colonyAddress: colonyAddress.toLowerCase(),
	extensionAddress: coinMachineClient.address.toLowerCase(),
	limit,
	},
	fetchPolicy: 'network-only',
	});

	const [extensionInitialised] = (
	subgraphData?.data?.extensionInitialisedEvents \|\| []
	).map(parseSubgraphEvent);

	/*
	* We use the `FromChain` suffix to make these events more easily
	* recognizable when reading the code
	*/
	const transferLogsFromChain = await getLogs(
	tokenClient,
	tokenClient.filters.Transfer(null, coinMachineClient.address, null),
	{
	fromBlock: extensionInitialised?.blockNumber,
	},
	);

Add total sold vs available widget #2898

Add total sold vs available widget #2898

Conversation

chinins commented Nov 16, 2021 • edited Loading

Description

TODO

ArmandoGraterol left a comment

Choose a reason for hiding this comment

arrenv commented Nov 24, 2021

arrenv left a comment

Choose a reason for hiding this comment

chinins commented Nov 25, 2021

arrenv left a comment

Choose a reason for hiding this comment

ArmandoGraterol left a comment

Choose a reason for hiding this comment

chinins commented Nov 25, 2021

arrenv left a comment

Choose a reason for hiding this comment

ArmandoGraterol left a comment

Choose a reason for hiding this comment

rdig left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chinins left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rdig left a comment

Choose a reason for hiding this comment

chinins commented Nov 16, 2021 •

edited

Loading