-
Notifications
You must be signed in to change notification settings - Fork 299
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Shayan/P2PS-1753/refactor exchange rates hook #11034
Merged
ali-hosseini-deriv
merged 66 commits into
binary-com:master
from
shayan-deriv:shayan/p2ps-1753/add-target-currency-when-calling-exchange-rates-endpoint
Nov 17, 2023
Merged
Changes from 65 commits
Commits
Show all changes
66 commits
Select commit
Hold shift + click to select a range
45c42b7
feat: adding new hook for exchange_rates [WIP]
shayan-deriv 16714fd
chore: removed unnecessary parts from the hook
b078952
feat: refactoring asset-summary [wip]
48edf35
Merge branch 'master' of github.com:shayan-deriv/deriv-app into shaya…
bcde04e
feat: adding global provider for handling subscription
b7f200b
build: added ts-usehooks to hooks package
e428816
chore: removed unused code
a727d31
revert: reverted changes in ExchangeRatesProvider
6177816
fix: updated the context to store rates in localstorage
a357728
refactor: refactored tradershub with the new hook
189117b
fix: fixed typescript error
8e1c8ba
chore: cleaned the code
cfa8422
fix: resolved merge conflicts
c5f1f76
refactor: refactored cashier transfer page
136c2b5
fix: resolved merge conflicts
0d43d6f
Merge branch 'binary-com:master' into shayan/p2ps-1753/add-target-cur…
shayan-deriv 71e7444
refactor: replace old exchange rate with the new one
niloofar-deriv ba4626e
refactor: added exchange rate provider
niloofar-deriv 0c59bf7
fix: removed extra imports
niloofar-deriv a98c5bb
refactor: added usep2pexchangerate hook for avoiding duplications
niloofar-deriv 841dbc8
test: added tests for usep2pexchangerate hook
niloofar-deriv 71a2d94
fix: fixed crypto-fiat-converter.spec.tsx errors
f33b606
fix: fixed test errors
66d20b6
Merge pull request #74 from niloofar-deriv/niloo/replace-p2p-exchange…
shayan-deriv a7df333
fix: resolved merge conflicts
shayan-deriv ac19f38
refactor: refactored percentage-selector component
shayan-deriv a6f315a
feat: added test for the new useExchangeRate
shayan-deriv 98a2e58
feat: added new test
shayan-deriv f9b570e
fix: resolved pr comment
shayan-deriv 1cac9a3
fix: resolved pr comment
shayan-deriv 688465e
fix: fixing cashier page crashing in test link
shayan-deriv 38bdcee
fix: trigger ci build
shayan-deriv 62a438a
Merge branch 'master' of github.com:shayan-deriv/deriv-app into shaya…
shayan-deriv f9ae4f2
build: updated package-lock.json
shayan-deriv 53b7a06
fix: fixed test failing
shayan-deriv fce2691
fix: p2p tests
niloofar-deriv 6c88cd2
Merge branch 'binary-com:master' into shayan/p2ps-1753/add-target-cur…
shayan-deriv abfaa24
fix: conditions
niloofar-deriv a150738
Merge pull request #75 from niloofar-deriv/niloo/fix-p2p-tests
shayan-deriv 5084c27
fix: floating rate tests
niloofar-deriv b963da7
Merge pull request #76 from niloofar-deriv/niloo/fix-p2p-tests
shayan-deriv f8f668b
Merge branch 'master' of github.com:shayan-deriv/deriv-app into shaya…
shayan-deriv bc084b3
Merge branch 'master' of github.com:shayan-deriv/deriv-app into shaya…
shayan-deriv 88758bb
fix: resolved pr comments
shayan-deriv 13bf723
Merge branch 'binary-com:master' into shayan/p2ps-1753/add-target-cur…
shayan-deriv 5f5f79e
fix: fixed test failing
shayan-deriv 9404a4d
fix: resolved pr comments
shayan-deriv e10a0d7
fix: minor fixes
shayan-deriv cd761df
revert: revert my latest changes for useTotalAccountBalance
shayan-deriv cb3241a
fix: bug fixed
shayan-deriv 559d465
fix: checking p2p issue
shayan-deriv 7fac83b
Merge branch 'binary-com:master' into shayan/p2ps-1753/add-target-cur…
shayan-deriv 6e31900
Merge branch 'master' of github.com:binary-com/deriv-app into shayan/…
shayan-deriv e7d12bd
Merge branch 'master' of github.com:binary-com/deriv-app into shayan/…
shayan-deriv 84eefa5
chore: removed console.log
shayan-deriv 7ef704e
fix: fixed crypto withdrawal issue
shayan-deriv 565fdeb
Merge branch 'master' of github.com:binary-com/deriv-app into shayan/…
shayan-deriv 4351f84
chore: removed asset-summary legacy code
shayan-deriv 94dbce6
chore: renamed some variables
shayan-deriv 6775a12
chore: removed legacy useExchangeRate and useTotalAccountBalance hook…
shayan-deriv eb255f2
refactor: removed legacy code for exchange_rate call
shayan-deriv 848570b
fix: fixing build error
shayan-deriv 898881b
test: removed my-ads-row-renderer test
shayan-deriv 1c41f62
chore: renamed the variables
shayan-deriv 71b5918
chore: removed unused code
shayan-deriv 4074579
fix: added new line at the end of package.json
shayan-deriv File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
shayan-deriv marked this conversation as resolved.
Show resolved
Hide resolved
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,13 +1,16 @@ | ||
import React from 'react'; | ||
import AppContent from './app-content'; | ||
import CashierProviders from './cashier-providers'; | ||
import { ExchangeRatesProvider } from '@deriv/stores'; | ||
|
||
type TProps = { passthrough: { root_store: React.ComponentProps<typeof CashierProviders>['store'] } }; | ||
|
||
const App: React.FC<TProps> = ({ passthrough: { root_store } }) => { | ||
return ( | ||
<CashierProviders store={root_store}> | ||
<AppContent /> | ||
<ExchangeRatesProvider> | ||
<AppContent /> | ||
</ExchangeRatesProvider> | ||
</CashierProviders> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hey @shayan-deriv |
||
); | ||
}; | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need to inject the provider in many places? isn't it defeat the purpose of having a provider?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we are only using it at the top level of each package which is
app.tsx
the other places are in the test files.first I tried to have the whole context and provider in
core
package but I faced many errors . that's why I moved it to hooks package.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wondering if there are any general rules/plans around using hooks vs using providers vs both for serving the data?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(just noticed that approach does not seem to be consistent across different parts of deriv-app)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wojciech-deriv so you mean instead of having the
useExchangeRate
for getting theexchange_rates
, we directly get them usinguseContext()
?I don't think there's is any general rules in the project.
for my case I first tried to just use a hook and handle the api call in it but we could not connect it to the component state and in some cases the
subscribe()
was not called. that's why @yashim-deriv suggested to use a context for handling the subscriptions( for now it's only handlingexchange_rates
call but we could make it more general for being used for al subscription calls)as far a I checked we are using the third approach ( providers + hook ) in the wallet project.
please check
APIProvider
,APIContext
, anduseAPI
inpackages/api
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc @markwylde-deriv