Mahdiyeh/ Refactor: Reports monorepo package #5620
Merged
Conversation
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
…_monorepo_package2
mahdiyeh-deriv
commented
Jun 29, 2022
| @@ -95,7 +97,11 @@ const Routes = ({ | |||
| }, []); | |||
|
|
|||
| React.useEffect(() => { | |||
| return () => onUnmountPortfolio(); | |||
| if (is_reports_url) { | |||
There was a problem hiding this comment.
since reports is not part of trader any more , when navigating from one of the reports routes to the trader, the trader component reloads and onUnmountPortfolio is going to be called. Adding this condition prevents this behavour.
balakrishna-deriv
previously approved these changes
Jul 1, 2022
matin-deriv
previously approved these changes
Jul 1, 2022
mahdiyeh-deriv
dismissed stale reviews from matin-deriv and balakrishna-deriv
via
bdc145e
…_monorepo_package2
mahdiyeh-deriv
commented
Jul 6, 2022
| @@ -44,6 +44,11 @@ const ContractUpdateForm = props => { | |||
| validation_errors, | |||
| } = contract; | |||
|
|
|||
| const [contract_profit_or_loss, setContractProfitOrLoss] = React.useState({ | |||
There was a problem hiding this comment.
changing input has lag because it's getting the value of inputs from contract. Adding local state for updating the view which is sync with contracts value improve this lag and fixes the performance issue.
matin-deriv
approved these changes
Jul 6, 2022
|
Kudos, SonarCloud Quality Gate passed!
|
likhith-deriv
pushed a commit
to likhith-deriv/deriv-app
that referenced
this pull request
Jul 17, 2022
* add initial config of reports monorepo * add config to core * fix event-source-polyfil version * resolve build errors * move validator to utils * fix trader files * delete reports, change imports * fix root_store error * fix chunkName * fix direcory path * fix jest errors * fix directory * make separate emptyPrtfolioMessage in trader * fix aliases * add translation * fix code smell * reload tests * fix empty-portfolio-message conflicts * remove unnecessary css files and functions * remove deriv-charts package and unnecessary related files * fix import from trader in reports * remove duplicated files * change BinaryLink to NavLink * rerun tests * fix circular dependency * fix EHOIST_PKG_VERSION error * fix test * remove position-drawer-card from trader * remove duplicated scss * remove bottom-widget * fix reports loader * add contract, portfolio and chartbarrier stores to the core * use core portfolio and contract store in trader and reports * move markers component, helpers and constants to the core * remove all duplicated files from trader and reports * remove contract store from core exports * update reports packages version * remove commented imports * add getContactValidationRules to contact core * move duration test to shared * fix active_symbols store * fix moduleLoader in routes.config error * fix stores constructor * move contract-type file to trader * move validation-rules to trading * remove serverTime from shared * remove .js from imports * remove all commented lines * fix error when the trade_store in localStorage in not existd yet * clean the code * fix is_multiplier error issue using it from portfolio store * add props type to open-positions and position-drawer * change portfolio file name * add action for updating contract_type observable in portfolio store * update with latest master * remove comments * add shared to translation scripts/add deriv/reports to the build * remove redundant comment in contract-replay.jsx * remove connectionWithContractUpdate from reports * fix openPositions default tab issue * fix to not remove closed contracts when navigating from reporst to trader * prevent unmounting the portfolio component when redirecting to reports packge * fix performance issue on profit_loss input * fix circular dependancy Co-authored-by: Carol Sachdeva <58209918+carol-binary@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Changes:
Please include a summary of the change and which issue is fixed below:
When you need to add unit test
When you need to add integration test
Test coverage checklist (for reviewer)
Type of change