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

[DTRA] Maryia/DTRA-739/Add tooltips for Take profit & Stop loss in ContractUpdateForm #13103

Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
96f8bb6
chore: add tooltips for take profit & stop loss
maryia-deriv Jan 23, 2024
d7a62a3
Merge branch 'master' of github.com:binary-com/deriv-app into maryia/…
maryia-deriv Jan 24, 2024
47570dc
Merge branch 'master' of github.com:binary-com/deriv-app into maryia/…
maryia-deriv Jan 24, 2024
c9273da
fix: styles and bug with apply button unnecessarily enabled
maryia-deriv Jan 24, 2024
bb9498e
fix: currency styles in input
maryia-deriv Jan 24, 2024
e73d3ab
chore: get rid of non-existing callback connectWithContractUpdate
maryia-deriv Jan 24, 2024
3331b9f
fix: total profit/loss update issue in contract update form on mobile
maryia-deriv Jan 24, 2024
19eb703
fix: tooltip styles for mobile
maryia-deriv Jan 24, 2024
0643438
fix: type setCurrentFocus
maryia-deriv Jan 24, 2024
ed0c015
revert: extra alignment of readme
maryia-deriv Jan 24, 2024
b6c2ce7
Merge branch 'master' of github.com:binary-com/deriv-app into maryia/…
maryia-deriv Jan 25, 2024
c85ece0
test: add tests
maryia-deriv Jan 25, 2024
54a1a15
fix: typo
maryia-deriv Jan 25, 2024
aedbefe
feat: take profit text unification
maryia-deriv Jan 26, 2024
d62021c
Merge branch 'master' of github.com:binary-com/deriv-app into maryia/…
maryia-deriv Jan 26, 2024
93551e4
chore: remove unused var
maryia-deriv Jan 26, 2024
0079495
Merge branch 'binary-com:master' into maryia/DTRA-739/tooltips-take-p…
maryia-deriv Jan 26, 2024
104793a
Merge branch 'binary-com:master' into maryia/DTRA-739/tooltips-take-p…
maryia-deriv Jan 26, 2024
247068e
Merge branch 'binary-com:master' into maryia/DTRA-739/tooltips-take-p…
maryia-deriv Feb 5, 2024
22275de
Merge branch 'binary-com:master' into maryia/DTRA-739/tooltips-take-p…
maryia-deriv Feb 6, 2024
2011e19
Merge branch 'binary-com:master' into maryia/DTRA-739/tooltips-take-p…
maryia-deriv Feb 6, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -29,24 +29,25 @@ describe('<AccumulatorCardBody />', () => {
setCurrentFocus: jest.fn(),
currency: 'USD',
removeToast: jest.fn(),
totalProfit: 111,
};
it('should display all contract card items, label, and values', () => {
render(<AccumulatorCardBody {...mock_props} />);
expect(screen.getByText('Initial stake:')).toBeInTheDocument();
expect(screen.getByText(getCardLabels().INITIAL_STAKE)).toBeInTheDocument();
expect(screen.getByText('123.00')).toBeInTheDocument();
expect(screen.getByText('Current stake:')).toBeInTheDocument();
expect(screen.getByText(getCardLabels().CURRENT_STAKE)).toBeInTheDocument();
expect(screen.getByText('234.00')).toBeInTheDocument();
expect(screen.getByText('Total profit/loss:')).toBeInTheDocument();
expect(screen.getByText(getCardLabels().TOTAL_PROFIT_LOSS)).toBeInTheDocument();
expect(screen.getByText('111.00')).toBeInTheDocument();
expect(screen.getByText('Take profit:')).toBeInTheDocument();
expect(screen.getByText(getCardLabels().TAKE_PROFIT)).toBeInTheDocument();
expect(screen.getByText('300.00')).toBeInTheDocument();
});

it('should display Take profit: label and - as value when take_profit is not available', () => {
if (mock_props?.contract_update?.take_profit?.order_amount)
mock_props.contract_update.take_profit.order_amount = null;
render(<AccumulatorCardBody {...mock_props} />);
expect(screen.getByText('Take profit:')).toBeInTheDocument();
expect(screen.getByText(getCardLabels().TAKE_PROFIT)).toBeInTheDocument();
expect(screen.getByText('-')).toBeInTheDocument();
});

Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,14 @@
import React from 'react';
import { configure, render, screen } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import { CONTRACT_TYPES, mockContractInfo, getCardLabels } from '@deriv/shared';
import { CONTRACT_TYPES, mockContractInfo, getCardLabels, isMobile } from '@deriv/shared';
import ContractUpdateForm from '../contract-update-form';

jest.mock('@deriv/shared', () => ({
...jest.requireActual('@deriv/shared'),
isMobile: jest.fn(() => false),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have to mock the deprecated isMobile in order to test the usage of ContractUpdateForm in mobile where it's rendering MobileWrapper. MobileWrapper is dependent on isMobile.

}));

const contract_info = mockContractInfo({
contract_id: 1,
contract_type: CONTRACT_TYPES.ACCUMULATOR,
Expand Down Expand Up @@ -43,11 +48,16 @@ describe('ContractUpdateForm', () => {
removeToast: jest.fn(),
setCurrentFocus: jest.fn(),
toggleDialog: jest.fn(),
totalProfit: 2.43,
};
const popoverTestid = 'dt_popover_wrapper';
beforeAll(() => {
el_modal.setAttribute('id', 'modal_root');
document.body.appendChild(el_modal);
});
afterEach(() => {
configure({ testIdAttribute: 'data-testid' });
});
afterAll(() => {
document.body.removeChild(el_modal);
});
Expand Down Expand Up @@ -165,8 +175,8 @@ describe('ContractUpdateForm', () => {
userEvent.type(take_profit_input, '5');
expect(new_props.contract.onChange).toHaveBeenCalled();
});
it(`should render unchecked Take profit & Stop loss inputs with checkboxes and disabled Apply button
for Multipliers when neither take profit, nor stop loss is selected or applied`, () => {
it(`should render unchecked Take profit & Stop loss checkboxes with popover icons, inputs and disabled Apply button
for Multipliers when neither take profit, nor stop loss is selected or applied in desktop`, () => {
const new_props = {
...mock_props,
contract: {
Expand All @@ -185,7 +195,27 @@ describe('ContractUpdateForm', () => {
const apply_button = screen.getByRole('button', { name: getCardLabels().APPLY });
expect(stop_loss_checkbox).not.toBeChecked();
expect(take_profit_checkbox).not.toBeChecked();
expect(screen.getAllByTestId(popoverTestid)).toHaveLength(2);
expect(inputs).toHaveLength(2);
expect(apply_button).toBeDisabled();
});
it('should render correct Total profit/loss, unchecked checkboxes without inputs & popover icons on mobile', () => {
(isMobile as jest.Mock).mockReturnValue(true);
const newProps = {
...mock_props,
contract: {
...contract,
contract_info: mockContractInfo({
...contract_info,
contract_type: CONTRACT_TYPES.MULTIPLIER.DOWN,
}),
},
is_accumulator: false,
};
render(<ContractUpdateForm {...newProps} isMobile />);
expect(screen.getByText(getCardLabels().TOTAL_PROFIT_LOSS)).toBeInTheDocument();
expect(screen.getByText('2.43 USD')).toBeInTheDocument();
expect(screen.getAllByTestId(popoverTestid)).toHaveLength(2);
expect(screen.queryByRole('textbox')).not.toBeInTheDocument();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ describe('MultiplierCardBody', () => {
setCurrentFocus: jest.fn(),
should_show_cancellation_warning: false,
toggleCancellationWarning: jest.fn(),
totalProfit: -0.44,
};
});

Expand All @@ -61,6 +62,7 @@ describe('MultiplierCardBody', () => {
};

it('should render the correct content for a Cancelled contract with Deal cancel.fee and negative Total profit/loss', () => {
// @ts-expect-error Check if error is gone after migrating MultiplierCardBody to TS
render(<MultiplierCardBody {...mock_props} />);

testCardContent();
Expand All @@ -76,7 +78,7 @@ describe('MultiplierCardBody', () => {
mock_props.contract_info.status = 'open';
mock_props.is_sold = false;
delete mock_props.contract_info.sell_price;

// @ts-expect-error Check if error is gone after migrating MultiplierCardBody to TS
render(<MultiplierCardBody {...mock_props} />);

testCardContent();
Expand All @@ -91,19 +93,22 @@ describe('MultiplierCardBody', () => {
delete mock_props.contract_info.cancellation;
delete mock_props.contract_info.sell_price;

// @ts-expect-error Check if error is gone after migrating MultiplierCardBody to TS
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Type error about MultiplierCardBody props type appeared here after I spread the props inside it. Needs to be checked after its TS migration.

render(<MultiplierCardBody {...mock_props} />);

expect(screen.getByText(mock_props.getCardLabels().NOT_AVAILABLE)).toBeInTheDocument();
expect(screen.getByText(progress_slider)).toBeInTheDocument();
});

it('should not render arrow indicator if the contract was sold (is_sold === true)', () => {
// @ts-expect-error Check if error is gone after migrating MultiplierCardBody to TS
render(<MultiplierCardBody {...mock_props} />);

expect(screen.queryByTestId('dt_arrow_indicator')).not.toBeInTheDocument();
});

it('should render arrow indicator if the contract is not sold (is_sold === false)', () => {
// @ts-expect-error Check if error is gone after migrating MultiplierCardBody to TS
render(<MultiplierCardBody {...mock_props} is_sold={false} />);

expect(screen.getAllByTestId('dt_arrow_indicator')).not.toHaveLength(0);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import React from 'react';
import ReactDOM from 'react-dom';
import { render, screen } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import { getCardLabels } from '@deriv/shared';
import ToggleCardDialog from '../toggle-card-dialog';

const contractUpdateForm = 'ContractUpdateForm';

jest.mock('../contract-update-form', () => jest.fn(() => <div>ContractUpdateForm</div>));
jest.mock('../../../icon', () => jest.fn((props: { icon: string }) => <div>{props.icon}</div>));

describe('ToggleCardDialog', () => {
const mockProps = {
addToast: jest.fn(),
contract_id: 1,
current_focus: null,
error_message_alignment: 'left',
getCardLabels: () => getCardLabels(),
getContractById: jest.fn(),
is_valid_to_cancel: false,
onMouseLeave: jest.fn(),
removeToast: jest.fn(),
setCurrentFocus: jest.fn(),
totalProfit: 50,
};
beforeAll(() => {
(ReactDOM.createPortal as jest.Mock) = jest.fn(component => {
return component;
});
});
afterAll(() => {
(ReactDOM.createPortal as jest.Mock).mockClear();
});
it('should render ContractUpdateForm when edit icon is clicked', () => {
render(<ToggleCardDialog {...mockProps} />);
expect(screen.queryByText(contractUpdateForm)).not.toBeInTheDocument();
const editIcon = screen.getByText('IcEdit');
userEvent.click(editIcon);
expect(screen.getByText(contractUpdateForm)).toBeInTheDocument();
});
it('should not render ContractUpdateForm when edit icon is clicked if is_valid_to_cancel === true', () => {
render(<ToggleCardDialog {...mockProps} is_valid_to_cancel />);
expect(screen.queryByText(contractUpdateForm)).not.toBeInTheDocument();
const editIcon = screen.getByText('IcEdit');
userEvent.click(editIcon);
expect(screen.queryByText(contractUpdateForm)).not.toBeInTheDocument();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ const contract_info = mockContractInfo({
describe('TurbosCardBody', () => {
const mock_props = {
addToast: jest.fn(),
connectWithContractUpdate: jest.fn(),
Copy link
Contributor Author

@maryia-deriv maryia-deriv Jan 24, 2024

Choose a reason for hiding this comment

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

connectWithContractUpdate is not declared anywhere, hence cleaning up all its usages.

contract_info,
contract_update: {
take_profit: {
Expand All @@ -37,6 +36,7 @@ describe('TurbosCardBody', () => {
setCurrentFocus: jest.fn(),
status: 'profit',
progress_slider_mobile_el: false,
totalProfit: 50,
};
beforeAll(() => {
(ReactDOM.createPortal as jest.Mock) = jest.fn(component => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import ArrowIndicator from '../../arrow-indicator';

type TAccumulatorCardBody = {
addToast: (toast_config: TToastConfig) => void;
connectWithContractUpdate?: React.ComponentProps<typeof ToggleCardDialog>['connectWithContractUpdate'];
contract_info: TContractInfo;
contract_update?: ContractUpdate;
currency: Required<TContractInfo>['currency'];
Expand All @@ -26,26 +25,20 @@ type TAccumulatorCardBody = {
is_sold: boolean;
onMouseLeave?: () => void;
removeToast: (toast_id: string) => void;
setCurrentFocus: (value: string) => void;
setCurrentFocus: (value: string | null) => void;
totalProfit: number;
is_positions?: boolean;
};

const AccumulatorCardBody = ({
addToast,
connectWithContractUpdate,
contract_info,
contract_update,
currency,
current_focus,
error_message_alignment,
getCardLabels,
getContractById,
indicative,
is_sold,
onMouseLeave,
removeToast,
setCurrentFocus,
is_positions,
...toggle_card_dialog_props
}: TAccumulatorCardBody) => {
const { buy_price, profit, limit_order, sell_price } = contract_info;
const { take_profit } = getLimitOrderAmount(contract_update || limit_order);
Expand Down Expand Up @@ -92,17 +85,10 @@ const AccumulatorCardBody = ({
{take_profit ? <Money amount={take_profit} currency={currency} /> : <strong>-</strong>}
{is_valid_to_sell && (
<ToggleCardDialog
addToast={addToast}
connectWithContractUpdate={connectWithContractUpdate}
contract_id={contract_info.contract_id}
current_focus={current_focus}
error_message_alignment={error_message_alignment}
getCardLabels={getCardLabels}
getContractById={getContractById}
is_accumulator
onMouseLeave={onMouseLeave}
removeToast={removeToast}
setCurrentFocus={setCurrentFocus}
{...toggle_card_dialog_props}
Copy link
Contributor Author

@maryia-deriv maryia-deriv Jan 24, 2024

Choose a reason for hiding this comment

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

I'm spreading all the props that are only passed through and not used in the component itself for Accumulators and Multipliers contract cards. They have been already spread in Turbos contract card.

/>
)}
</ContractCardItem>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import React from 'react';
import classNames from 'classnames';
import { isCryptocurrency, getIndicativePrice, getCurrentTick, getDisplayStatus } from '@deriv/shared';
import { isCryptocurrency, getIndicativePrice, getCurrentTick, getDisplayStatus, getTotalProfit } from '@deriv/shared';
import ContractCardItem from './contract-card-item';
import CurrencyBadge from '../../currency-badge';
import DesktopWrapper from '../../desktop-wrapper';
Expand All @@ -25,7 +25,6 @@ export type TContractCardBodyProps = {

const ContractCardBody = ({
addToast,
connectWithContractUpdate,
contract_info,
contract_update,
currency,
Expand Down Expand Up @@ -68,13 +67,13 @@ const ContractCardBody = ({

const toggle_card_dialog_props = {
addToast,
connectWithContractUpdate,
current_focus,
error_message_alignment,
getContractById,
onMouseLeave,
removeToast,
setCurrentFocus,
totalProfit: is_multiplier && !isNaN(Number(profit)) ? getTotalProfit(contract_info) : Number(profit),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

bug fix: Passing the totalProfit as a prop is essential to keep 'Total profit/loss' value always updated in mobile contract update form. This issue is that currently it's getting updated only when a user interacts with the form.

};

let card_body;
Expand Down
Loading
Loading