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.
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
SmartTrader Redesign #660
SmartTrader Redesign #660
Changes from 70 commits
8c5a82d
2310f3b
76acf5e
76462d1
86363ae
afb2a98
e850770
cb052f2
8233c67
f2e85ba
361a172
35477b9
a9f1c18
669e559
d2ca4cd
36bc4cf
32d0587
16bb30c
dcf07cd
bd3a615
17556e0
21a08cd
e28b94f
e1a4143
992b154
855796f
0d6f3ea
660818b
38fa3e4
06c448a
9ec72cb
80ed9f6
03f4c88
8e9041b
11c4887
56b86f2
52dfe3a
63c7700
23627e9
75051de
84b4f59
ff83f05
43ef329
f57b58a
c600bd1
d5d5a30
646c9bc
8ea7893
fd10aea
c41e4eb
666e227
0551228
ec20aa3
8069861
256162a
bc80ae2
b25339d
32c94fe
ddf7892
74c5be4
b5a9145
a834f06
dca3000
6754dc6
7cc0b92
4ac05f5
89ce4d8
5e80046
51b7251
17b869e
e905c6d
fa6f350
14fea7a
a55678e
5602639
214ea56
9ab43f1
068b4f3
4040ddc
05a7eb2
d6de3fe
efc4598
4ef9b9d
be46503
453dd8f
9b88e2d
073eafe
7273be8
3f9058e
3eb3e7e
761dcbe
35871d0
31b23db
70793ea
68ef580
d6758db
44fd237
9d53f1e
d45fd8d
249b7ec
dd87956
92d1f49
b6563fe
dc685ea
c6b9022
56e07e6
a832e61
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
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.
please explain why was this approach selected to store contract data in the global
window
object? same forwindow.trade
&window.purchase
.Why not
localStorage
or some custom store accessible from any point of application if you're trying to avoid circular dependency?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.
These data are only available in the current session, so upon browser refresh we want these data to go away. We are leveraging on global variables to observe and fully use the state features of react.
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.
But if you want these data to do away on browser refresh, why not sessionStorage in this case?
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.
session storage doesn't go away after refresh.
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.
@prince-deriv seems the
contract
object is only used in this file. Is there any reason we should set it in window?Can we set it as a class member object? or, use a separate context object just for
contract
?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 refactoring this to use class member, since the QA is almost done with testing and we no longer need the window data for testing.