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

refactor: ♻️ migrated code to TS #45

Conversation

likhith-deriv
Copy link
Owner

Changes:

Please provide a summary of the change.

Screenshots:

Please provide some screenshots of the change.

@coveralls
Copy link

coveralls commented Jul 19, 2023

Pull Request Test Coverage Report for Build 5832298144

  • 4 of 4 (100.0%) changed or added relevant lines in 3 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.001%) to 9.795%

Files with Coverage Reduction New Missed Lines %
packages/account/src/Configs/terms-of-use-config.ts 1 0%
Totals Coverage Status
Change from base Build 5820225697: -0.001%
Covered Lines: 9837
Relevant Lines: 84006

💛 - Coveralls


/*
* This component is used with Formik's Field component.
*/
const CheckboxField = ({ field: { name, value, onChange }, id, label, className, ...props }) => {
const CheckboxField = ({ field: { name, value, onChange }, id, label, className, ...props }: TCheckboxFieldProps) => {

Choose a reason for hiding this comment

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

If this component is being used with formik Field component, can we try wrapping this with Field directly and we'll be able to remove this field prop?

Copy link
Owner Author

@likhith-deriv likhith-deriv Aug 7, 2023

Choose a reason for hiding this comment

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

Can u please elaborate?
Checkbox field is passed as a component to Field
<Field component={CheckboxField}

@@ -5,7 +5,7 @@ import { Text } from '@deriv/components';

export const Hr = () => <div className='terms-of-use__hr' />;

export const BrokerSpecificMessage = ({ target }) => (
export const BrokerSpecificMessage = ({ target }: { target: string }) => (

Choose a reason for hiding this comment

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

as we know the possible values of target, can we try writing an enum for it with just these possible 5 values?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Will get confirmation on the active broker codes then change to enum

@@ -4,6 +4,7 @@
flex-grow: 1;
margin: 0 8rem !important;
width: 84% !important;
padding-bottom: unset;

Choose a reason for hiding this comment

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

since we set the padding-bottom at the top level, isn't it available on mobile as well as there's no override?
Am I missing something?

className={cn('details-form__elements', 'terms-of-use')}
style={{ paddingBottom: isDesktop() ? 'unset' : null }}
>
<div className={cn('details-form__elements', 'terms-of-use')}>
Copy link

@shahzaib-deriv shahzaib-deriv Jul 31, 2023

Choose a reason for hiding this comment

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

since we set the padding-bottom at the top level, isn't it available on mobile as well as there's no override?
Am I missing something?

Copy link
Owner Author

Choose a reason for hiding this comment

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

unset or null does the same as there is no override available

Copy link
Collaborator

Choose a reason for hiding this comment

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

can we change this classnames import to commonly used in our app? :)

@github-actions
Copy link

github-actions bot commented Aug 7, 2023

Generating Lighthouse report...

@likhith-deriv likhith-deriv force-pushed the likhith/WEBREL-759/refactor-tnc-component branch from febf750 to 8c6103b Compare August 10, 2023 11:05
…nto likhith/WEBREL-759/refactor-tnc-component
};

type TTermsOfUseProps = {
getCurrentStep: () => number;
Copy link
Collaborator

Choose a reason for hiding this comment

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

the list is not huge but anyway, wouldn’t it be better to sort all fields alphabetically? :)

Co-authored-by: yauheni-deriv <103182683+yauheni-deriv@users.noreply.github.com>
@likhith-deriv likhith-deriv merged commit 5ff8939 into likhith/WEBREL-795/Refactor-real-account-signup-flow Aug 11, 2023
2 checks passed
likhith-deriv added a commit that referenced this pull request Oct 5, 2023
* fix: ⚡ refactored Address-details component

* fix: 🎨 failing testcases

* refactor: ♻️ incorporated review comments

* feat: 🔥 removed PlatformContext

* fix: testcase failure

* refactor: ♻️ incorporated review comments

* refactor: ♻️ refactor component to remove unwanted code (#44)

* refactor: ♻️ refactor component to remove unwanted code

* feat: removed commented code

* test: ✅ added testcase

* test: 🧪 added tests

* refactor: ♻️ incorporated review comments

* refactor: ♻️ incorporated review comments

* refactor: ♻️ incorporated review comments

* refactor: ♻️ migrated code to TS (#45)

* refactor: ♻️ migrated code to TS

* refactor: ♻️ incorporated review comments

* refactor: ♻️ incorporated review comments

* Update packages/account/src/Types/common-prop.type.ts

Co-authored-by: yauheni-deriv <103182683+yauheni-deriv@users.noreply.github.com>

---------

Co-authored-by: yauheni-deriv <103182683+yauheni-deriv@users.noreply.github.com>

* refactor: ⚡ refactored currency selector component (#43)

* refactor: ⚡ refactored currency selector component

* fix: 🚨 lint errors

* feat: incorporated store values

* fix: 🎨 split code into small components

* fix: 🎨 removed un-necessary array

* refactor: ♻️ incorporated review comments

* refactor: ♻️ incorporated review comments

* resolved failing test cases

* chore: incorporated review comment

* fix: build issue

* fix: build issue

* ref: incorporated review comments

* refactor: Modified code to contain hooks

* refactor: Modified code to contain hooks

* replaced test component

* fix: eslint issues

* fix: failing testcases

* ref: Incorporated review somments

* Likhith/webrel 780/migrate Email and password section (#7)

* chore: migrated deriv-email and unlink component

* chore: migrate password-platform to tsx

* chore: migrate password-platform to tsx

* chore: added missing test cases

* ref: incorporated review comments

* refactor: incorporated review comments

* refactor: incorporated review comments

* refactor: incorporated review comments

* ref: Incorporated review somments

* chore: config files ts migration and refactor (#8)

* chore: config files ts migartion and refactor

* refactor: test case for config func

* fix: config typing

* refactor: remove custom accounts residence type

* refactor: review comment

* chore: added test cases

* fix: revert boolean in types

* refactor: default value fields

* refactor: files types, import react types

* refactor: react types

* fix: condition rule

* fix: failing testcase

---------

Co-authored-by: “yauheni-kryzhyk-deriv” <“yauheni@deriv.me”>

* fix: incorporated review comments

* fix: failing testcase

* refactor: migrate form-fields file (#10)

* refactor: migrate form-fields file

* refactor: migrate form-fields file

* ref: incorporated review comments

* fix: failing testcase

* fix: incorporated review comments

* fix: incorporated review comments

* fix: incorporated review comments

* fix: resolved Sonar lint errors

* fix: tslint issues

* fix: review comments

* fix: review comments

* refactor: 🎨 incorporated types for test cases

* fix: 🦺 incorporated review comments

* fix: 🎨 incorporated ui-store-fix

* fix: ⚰️ replaced deprecated hooks

* fix: 🔥 unused dependency

* Merge branch 'master' into KYC-accounts-package-TS-migration--test-coverage/sprint-8

* chore: added package-lock

* fix: added value for mock-store

---------

Co-authored-by: yauheni-deriv <103182683+yauheni-deriv@users.noreply.github.com>
Co-authored-by: “yauheni-kryzhyk-deriv” <“yauheni@deriv.me”>
Co-authored-by: Shahzaib <shahzaib@deriv.com>
likhith-deriv pushed a commit that referenced this pull request Dec 12, 2023
…ctoring (binary-com#10848)

* chore: empty commit

* Kate / DTRA-325 / TS migration: Form/TradeParams/Duration files in Trader package (#43)

* refactor: ts of expity text

* refactor: ts of duration toggle

* refactor: ts of simple duration

* fix: types

* feat: ts of duration wrapper

* refactor: ts of daration

* refactor: add ts for advanced duration

* refactor: ts of duration mobile

* refactor: unit test

* refactor: apply suggestions

* chore: add fallback for simple duration unit

* refactor: apply suggestions

* chore: refactor type

* refactor: remove code smells

* Kate / DTRA-421 / TS migration: Contract + ContractReplay in Trader package (#44)

* refactor: ts of contract file

* refactor: ts of contract replay

* refactor: add more types for contract store

* chore: expand type

* refactor: apply suggestions

* refactor: separate replay chart from contract replay

* refactor: add ternary rendering

* fix: revert latest changes

* refactor: remove code smells

* Maryia/DTRA-324/refactor: TS migration of trade.jsx and refactoring (#45)

* chore: migrate trade to ts

* chore: remove unnecessary line

* chore: migrate to ts - test and init-store

* chore: remove unused test component

* chore: more files to ts

* chore: add types to trade

* chore: trade-chart to ts

* refactor: is_mobile

* chore: add types to SmartChartSwitcher

* chore: ToolbarWidgetsBeta to ts

* fix: a type

* fix: ws type

* chore: remove unused setting

* refactor: alphabetical order

* chore: index.js to index.ts

* refactor: address comments

* chore: remove code smells

* chore: change type

* refactor: apply suggestions

* fix: add missing string

* refactor: apply suggestions

* chore: add math trunc method

* chore: remove extra ui mocked prop

---------

Co-authored-by: Maryia <103177211+maryia-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.

4 participants