Skip to content
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

henry/dtra-356/fix: ts-migration-digitsJSX #24

Conversation

henry-deriv
Copy link

Changes:

Please provide a summary of the change.

Screenshots:

Please provide some screenshots of the change.

@henry-deriv henry-deriv marked this pull request as draft August 21, 2023 13:19
@henry-deriv henry-deriv changed the title fix: initial commit henry/dtra-356/fix: ts-migration-digitsJSX Aug 21, 2023
@henry-deriv henry-deriv marked this pull request as ready for review August 22, 2023 03:16
@coveralls
Copy link

coveralls commented Aug 22, 2023

Pull Request Test Coverage Report for Build 5933991430

  • 1 of 12 (8.33%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 10.12%

Changes Missing Coverage Covered Lines Changed/Added Lines %
packages/shared/src/utils/helpers/logic.ts 0 2 0.0%
packages/trader/src/Modules/Contract/Components/Digits/digits.tsx 1 10 10.0%
Totals Coverage Status
Change from base Build 5926779536: 0.0%
Covered Lines: 10151
Relevant Lines: 83763

💛 - Coveralls

@github-actions
Copy link

Generating Lighthouse report...

const isMounted = useIsMounted();

const { contract_info, digits_array, is_digit_contract, is_trade_page, underlying } = props;

const onChangeStatus = params => {
const onChangeStatus: React.ComponentProps<typeof DigitsWrapper>['onChangeStatus'] = params => {
Copy link
Owner

@kate-deriv kate-deriv Aug 22, 2023

Choose a reason for hiding this comment

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

Can't we use just TOnChangeStatus? Same for TOnLastDigitSpot. You declare those types upper and used inside DigitsWrapper

Copy link
Author

Choose a reason for hiding this comment

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

lol yeah same same. Doesnt make a difference

const [is_selected_winning, setIsSelectedWinning] = React.useState<boolean>();
const [is_latest, setIsLatest] = React.useState<boolean>();
const [is_won, setIsWon] = React.useState<boolean>();
const [is_lost, setIsLost] = React.useState<boolean>();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
const [is_lost, setIsLost] = React.useState<boolean>();
const [is_lost, setIsLost] = React.useState<boolean | undefined>();

@henry-deriv initial value is undefined, is TS not complaining about it?

Copy link
Author

Choose a reason for hiding this comment

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

not complaining since it can detect the type i guess

const setIsLost: React.Dispatch<React.SetStateAction<boolean | undefined>>

@@ -548,6 +548,13 @@ type TContractStore = {
contract_update_take_profit: string;
has_contract_update_stop_loss: boolean;
has_contract_update_take_profit: boolean;
last_contract: {
Copy link
Collaborator

Choose a reason for hiding this comment

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

@henry-deriv I don't really understand why this TContractStore type includes a mix of properties from 2 stores: contract-store and contract-trade-store. last_contract doesn't exist in contract-store, and has_contract_update_stop_loss doesn't exist in contract-trade-store. i will be also changing this in my PR, it would be correct to separate them. what do you think?

Copy link
Author

@henry-deriv henry-deriv Aug 22, 2023

Choose a reason for hiding this comment

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

its due to this.contract_store = new ContractStore(this.root_store, { contract_id }); inside contract-replay-store

Copy link
Author

@henry-deriv henry-deriv Aug 22, 2023

Choose a reason for hiding this comment

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

contract-replay-store is essentially its' own props plus the whole contractstore

Copy link
Author

@henry-deriv henry-deriv Aug 22, 2023

Choose a reason for hiding this comment

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

we also have this in contract-trade-store

const contract = new ContractStore(this.root_store, { contract_id });

this.contracts.push(contract);

this.contracts pretty much contains multiple of contractStore instantiation with its own contract_id

Copy link
Author

Choose a reason for hiding this comment

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

Thats why contract-trade-store and contract-replay-store have states that are not shared between them and shared between each other at the same time lol

Copy link
Author

Choose a reason for hiding this comment

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

yeah i was just saying contract-replay-store and contract-trade-store have instantiations of contract-store.

And thats the reason why we have mix of properties/states in all three stores.

Copy link
Owner

Choose a reason for hiding this comment

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

If you don't mind it will be better to merge Henry's branch now and then fix it in your PR in order to avoid conflicts

Copy link
Author

Choose a reason for hiding this comment

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

I don't really understand why this TContractStore type includes a mix of properties from 2 stores: contract-store and contract-trade-store. last_contract doesn't exist in contract-store, and has_contract_update_stop_loss doesn't exist in contract-trade-store. i will be also changing this in my PR, it would be correct to separate them. what do you think?

Answer to this whoever put it in was probably confused 😅

Copy link
Collaborator

Choose a reason for hiding this comment

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

there will be conflicts anyway :), and i'll need to fix them . let's do this 👍🏻

Copy link
Author

Choose a reason for hiding this comment

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

Yes lets separate them and type them properly 👍

@kate-deriv kate-deriv merged commit bca1185 into kate-deriv:kate/ts_migration_trader_package_4 Aug 22, 2023
1 check passed
kate-deriv added a commit that referenced this pull request Oct 4, 2023
* fix: ts-migrate trade-params

* fix: sonarcloud

* fix: pull in changes from tech debt 2 package

* fix: resolve comments

* fix: move type to common prop types

* fix: move type to common prop types

* fix: move type to common prop types file

* fix: resolve comments

* Kate / DTRA-321 / TS migration of trade-params and trade-params-mobile (#6)

* refactor: migrate trade params and started mobile version

* refactor: ts migration of trade params mobile

* chore: add nessasary prop

* refactor: apply suggestions

* chore: change todo text

* refactor: add import

* fix: circleCI error

* fix: ts migrate trade-footer-extensions.jsx

* fix: fix import

* fix: remove progress-slider-stream since its not used

* fix: resolve comments

* fix: reset with master

* fix: reset with master

* fix: reset with master

* fix: reset with master

* fix: circleCI

* fix: togglePositions folder

* fix: dtra-346 marketisclosed and marketcountdowntimer migration

* fix: bug

* Maryia/dtra-270/TS migration: trade-store (#5)

* chore: prepare for migration

* chore: add more types to trade-store

* chore: add more types to trade-store

* chore: add more types to trade-store

* chore: add more types to trade-store

* chore: migrated trade-store to ts

* chore: improve types in trade-store

* fix: type

* revert: hooks package installation

* refactor: address review

* fix: resolve comments

* feat: add ts migartion of store

* refactor: add prev changes

* chore: empty commit

* fix: add lost mocked

* fix: resolve comments

* Kate / DTRA-354 / Components/Form/Purchase files in Trader package (#21)

* refactor: ts migartion of purchase files

* refactor: remove duplicated types

* refactor: apply suggestions

* maryia/fix: sonarcloud warnings (#7)

* fix: sonarcloud warnings

* fix: Unexpected end of JSON input

* fix: bug

* fix: sonarcloud

* fix: reorder props

* fix: test cases

* fix: coveralls

* fix: coveralls

* fix: this component doesnt exist anymore, hence test was also removed

* henry/dtra-356/fix: ts-migration-digitsJSX (#24)

* fix: initial commit

* fix: ts migrate digits JSX

* fix: small type change

* fix: comment

* chore: removed unused state

* Maryia/dtra-355/Migrate ContractDrawer files to TS (#22)

* feat: migrated swipeable-components to TS

* feat: migrated market-closed-contract-overlay & index to TS

* chore: migrated contract-drawer-card.tsx to ts

* build: fix type

* chore: migrated contract-drawer to ts

* chore: fixed existing types in digits and logic

* chore: sort types properties in alphabetical order

* Kate/dtra 357/ts contract audit files (#23)

* refactor: apply suggestion from prev pr

* refactor: start ts migration of contract audit

* chore: change comment

* refactor: ts of contract audit item

* refactor: ts migration of contract details

* refactor: ts migration of contract history

* refactor: add preprepared types

* refactor: tests

* chore: apply nit

* refactor: apply suggestions

* refactor: apply suggestions

* chore: fix of sonar cloud

* Maryia/dtra-373/remove localize from ContractCardHeader component (#25)

* refactor: remove localize from contract-card-header

* refactor: use Localize component instead of localize helper

* build: install RTL deps in shared package

* fix: sonar cloud codesmells

* fix: build TS errors (#32)

* refactor: tests

* fix: more conflicts

* chore: empty commit

* maryia/fix: type issues on package 4 (#41)

* fix: type issues

* chore: file change from package 3

* chore: keep small changes from package 3

* chore: removed unnecessary todo comment

* refactor: apply suggestions

* fix: types in tests

* fix: test extention

* fix: wallet file

* fix: add turbos to types

---------

Co-authored-by: Henry Hein <henry@regentmarkets.com>
Co-authored-by: Maryia <103177211+maryia-deriv@users.noreply.github.com>
Co-authored-by: henry-deriv <118344354+henry-deriv@users.noreply.github.com>
kate-deriv added a commit that referenced this pull request Oct 11, 2023
* fix: ts-migrate trade-params

* fix: sonarcloud

* fix: pull in changes from tech debt 2 package

* fix: resolve comments

* fix: move type to common prop types

* fix: move type to common prop types

* fix: move type to common prop types file

* fix: resolve comments

* Kate / DTRA-321 / TS migration of trade-params and trade-params-mobile (#6)

* refactor: migrate trade params and started mobile version

* refactor: ts migration of trade params mobile

* chore: add nessasary prop

* refactor: apply suggestions

* chore: change todo text

* refactor: add import

* fix: circleCI error

* fix: ts migrate trade-footer-extensions.jsx

* fix: fix import

* fix: remove progress-slider-stream since its not used

* fix: resolve comments

* fix: reset with master

* fix: reset with master

* fix: reset with master

* fix: reset with master

* fix: circleCI

* fix: togglePositions folder

* fix: dtra-346 marketisclosed and marketcountdowntimer migration

* fix: bug

* Maryia/dtra-270/TS migration: trade-store (#5)

* chore: prepare for migration

* chore: add more types to trade-store

* chore: add more types to trade-store

* chore: add more types to trade-store

* chore: add more types to trade-store

* chore: migrated trade-store to ts

* chore: improve types in trade-store

* fix: type

* revert: hooks package installation

* refactor: address review

* fix: resolve comments

* feat: add ts migartion of store

* refactor: add prev changes

* chore: empty commit

* fix: add lost mocked

* fix: resolve comments

* Kate / DTRA-354 / Components/Form/Purchase files in Trader package (#21)

* refactor: ts migartion of purchase files

* refactor: remove duplicated types

* refactor: apply suggestions

* maryia/fix: sonarcloud warnings (#7)

* fix: sonarcloud warnings

* fix: Unexpected end of JSON input

* fix: bug

* fix: sonarcloud

* fix: reorder props

* fix: test cases

* fix: coveralls

* fix: coveralls

* fix: this component doesnt exist anymore, hence test was also removed

* henry/dtra-356/fix: ts-migration-digitsJSX (#24)

* fix: initial commit

* fix: ts migrate digits JSX

* fix: small type change

* fix: comment

* chore: removed unused state

* Maryia/dtra-355/Migrate ContractDrawer files to TS (#22)

* feat: migrated swipeable-components to TS

* feat: migrated market-closed-contract-overlay & index to TS

* chore: migrated contract-drawer-card.tsx to ts

* build: fix type

* chore: migrated contract-drawer to ts

* chore: fixed existing types in digits and logic

* chore: sort types properties in alphabetical order

* Kate/dtra 357/ts contract audit files (#23)

* refactor: apply suggestion from prev pr

* refactor: start ts migration of contract audit

* chore: change comment

* refactor: ts of contract audit item

* refactor: ts migration of contract details

* refactor: ts migration of contract history

* refactor: add preprepared types

* refactor: tests

* chore: apply nit

* refactor: apply suggestions

* refactor: apply suggestions

* chore: fix of sonar cloud

* Maryia/dtra-373/remove localize from ContractCardHeader component (#25)

* refactor: remove localize from contract-card-header

* refactor: use Localize component instead of localize helper

* build: install RTL deps in shared package

* feat: merge previous tech debt branch

* fix: sonar cloud codesmells

* Merge branch kate/ts_migration_trader_package_4 into kate/ts_migration_trader_package_5

* Kate / DTRA-378 / TS migration: Multiplier components and containers in Trader package (#26)

* refactor: ts of cancel deal and risk managment info

* fix: types in dialog tsx

* refactor: start ts of multiplier options

* refactor: ts of mult amount modal

* refactor: remove default val from basis component

* fix: type of amount mobile

* refactor: applied suggestions

* refactor: ts of multiplier info (#28)

* Henry/dtra 376/fix: ts migration positionsdrawer folder (#27)

* fix: positions drawer folder

* fix: coveralls

* fix: where is my commit

* fix: found commits

* fix: file rename

* fix: comments

* fix: comments

* fix: correct logic

* fix: remove unused props

* fix: types

* refactor: remove code mells

* fix: ts-migrate-populate header (#30)

* Maryia/DTRA-377/TS migration: TradingDatePicker + TradingTimePicker + TimePicker + Dialog (#29)

* chore: timepicker to ts

* chore: trading timepicker to ts

* chore: add types for TradingDatePicker & DatePicker

* refactor: added default values

* refactor: remove code small

* chore: empty commit

* fix: not using index as key (#31)

* fix: ternary bug

* fix: build TS errors (#32)

* refactor: apply suggestions

* refactor: change type of time

* chore: empty commit

* refactor: tests

* fix: conflicts

* fix: conflicts

* fix: tests

* fix: types in tests

* fix: test extention

* fix: add turbos to types back

* fix: add backup for target name

* fix: revert empty strings

---------

Co-authored-by: Henry Hein <henry@regentmarkets.com>
Co-authored-by: Maryia <103177211+maryia-deriv@users.noreply.github.com>
Co-authored-by: henry-deriv <118344354+henry-deriv@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants