Skip to content

Add new precision filter to RPC component#4008

Merged
DrW3RK merged 6 commits intomasterfrom
#3986
Nov 16, 2022
Merged

Add new precision filter to RPC component#4008
DrW3RK merged 6 commits intomasterfrom
#3986

Conversation

@alfarok
Copy link
Copy Markdown
Contributor

@alfarok alfarok commented Nov 4, 2022

This PR addresses #3986 and replaces the hard-coded values in #3954.

This issue was caused by the use of the HumanReadable filter which was rounding the total values to zero due to a lack of precision in supporting smaller values. To address the issue we add a new Precise filter that still adequately converts the chain values to the appropriate denomination, but avoids the toFixed() invocation.

Before:
image

After:
image

@alfarok alfarok added the A2 - Please Review Pull request is ready for review. label Nov 4, 2022
@alfarok alfarok self-assigned this Nov 4, 2022
@alfarok alfarok requested a review from emresurmeli November 4, 2022 15:00
@emresurmeli
Copy link
Copy Markdown
Contributor

Nice - this works. Would it be possible to inject some react to actually get the answer for the multiplication?

@alfarok
Copy link
Copy Markdown
Contributor Author

alfarok commented Nov 7, 2022

@emresurmeli yes it is totally possible but may be a little tricky. One approach would be to create a RPCMath component that wraps two child RPC components and receives an operator prop to apply, something like:

<RPCMath operator="*">
  <RPC id="inputA" visible=false />
  <RPC id="inputB" visible=false />
</RPCMath>

You would need to prevent the children from rendering anything, making sure only the result gets rendered. This is why I added the visible prop which could default to true but be overridden in scenarios like this.

If the team thinks something like this would be useful or we find several existing scenarios where this applications could replace hard-coded values we can outline it in a new task.

Until this is possible we can also revert the math example to display fixed values in order to display a result if you think it adds clarity.

@alfarok
Copy link
Copy Markdown
Contributor Author

alfarok commented Nov 7, 2022

The more challenging aspect would be handling the events sequence. The component should run initially using the default values and run again when both RPC invocations in the child components are complete.

@alfarok
Copy link
Copy Markdown
Contributor Author

alfarok commented Nov 9, 2022

Hey @emresurmeli, I restored the hard-coded example. I think this one should be ready for another look whenever you have a chance.

Copy link
Copy Markdown
Member

@DrW3RK DrW3RK left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! Thank you @alfarok

@DrW3RK DrW3RK merged commit c253756 into master Nov 16, 2022
@DrW3RK DrW3RK deleted the #3986 branch November 16, 2022 13:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A2 - Please Review Pull request is ready for review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants