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]Ahmad/ Hour Picker and Other fixes #17168

Open
wants to merge 18 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 10 commits
Commits
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 @@ -6,6 +6,8 @@ import { mockStore } from '@deriv/stores';
import { TCoreStores } from '@deriv/stores/types';
import userEvent from '@testing-library/user-event';
import { useSnackbar } from '@deriv-com/quill-ui';
import moment from 'moment';
Copy link
Contributor

@wojciech-deriv wojciech-deriv Oct 15, 2024

Choose a reason for hiding this comment

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

in wallets, we recently removed moment due to its size,
as much as I like moment (and I personally still use it in non-client-facing projects), I'm wondering if it shouldn't be deprecated and not being added anymore, in favour of something more modern and more treeshakeable

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In V2 , we are not actually using moment . The server_time is handled in store primarily used in V1 and is handled via moment . So in order to utilize that I had to use the moment .

import { toMoment } from '@deriv/shared';

global.ResizeObserver = jest.fn().mockImplementation(() => ({
observe: jest.fn(),
Expand Down Expand Up @@ -35,7 +37,7 @@ jest.mock('@deriv/shared', () => ({
...jest.requireActual('@deriv/shared'),
toMoment: jest.fn(() => ({
clone: jest.fn(),
isSame: jest.fn(),
isSame: jest.fn(() => true),
})),
}));

Expand Down Expand Up @@ -64,6 +66,9 @@ describe('Duration', () => {
symbol: 'EURUSD',
},
},
common: {
server_time: moment('2024-10-10T11:23:10.895Z'),
},
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { TCoreStores } from '@deriv/stores/types';
import TraderProviders from '../../../../../trader-providers';
import userEvent from '@testing-library/user-event';
import { ContractType } from 'Stores/Modules/Trading/Helpers/contract-type';
import moment from 'moment';

global.ResizeObserver = jest.fn().mockImplementation(() => ({
observe: jest.fn(),
Expand Down Expand Up @@ -81,6 +82,9 @@ describe('DurationActionSheetContainer', () => {
symbol: '1HZ100V',
},
},
common: {
server_time: moment('2024-10-10T11:23:10.895Z'),
},
});
});

Expand Down Expand Up @@ -244,19 +248,8 @@ describe('DurationActionSheetContainer', () => {

it('should show End Time Screen on selecting the days unit', () => {
renderDurationContainer(default_trade_store, 'd');

const date_input = screen.getByTestId('dt_date_input');
const time_input = screen.getByDisplayValue('23:59:59 GMT');
expect(date_input).toBeInTheDocument();
expect(time_input).toBeInTheDocument();
});

it('should open timepicker on clicking on time input in the days page', () => {
renderDurationContainer(default_trade_store, 'd');
const time_input = screen.getByDisplayValue('23:59:59 GMT');
expect(time_input).toBeInTheDocument();
userEvent.click(time_input);
expect(screen.getByText('Pick an end time'));
});

it('should open datepicker on clicking on date input in the days page', () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { ActionSheet, Text, TextField } from '@deriv-com/quill-ui';
import { ActionSheet, Text, TextField, useSnackbar } from '@deriv-com/quill-ui';
import { LabelPairedCalendarSmRegularIcon, LabelPairedClockThreeSmRegularIcon } from '@deriv/quill-icons';
import { Localize } from '@deriv/translations';
import React, { useEffect, useState } from 'react';
Expand All @@ -14,7 +14,7 @@ import {
getDatePickerStartDate,
getProposalRequestObject,
} from 'AppV2/Utils/trade-params-utils';
import { useDtraderQuery } from 'AppV2/Hooks/useDtraderQuery';
import { invalidateDTraderCache, useDtraderQuery } from 'AppV2/Hooks/useDtraderQuery';
import { ProposalResponse } from 'Stores/Modules/Trading/trade-store';

const timeToMinutes = (time: string) => {
Expand All @@ -40,6 +40,9 @@ const DayInput = ({
const [current_gmt_time, setCurrentGmtTime] = React.useState<string>('');
const [open, setOpen] = React.useState(false);
const [open_timepicker, setOpenTimePicker] = React.useState(false);
const [trigger_date, setTriggerDate] = useState(false);
const [is_disabled, setIsDisabled] = useState(false);
const [temp_end_date, setTempEndDate] = useState(end_date);
const { common } = useStore();
Copy link
Contributor

Choose a reason for hiding this comment

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

temp_end_date? what does that name mean?
could it be just "end_date", or is the reason for it to be "temp_"? (feels bad to put "temp" things to state, but there might be reasons for it which I'm not seeing at the moment)

Copy link
Contributor

Choose a reason for hiding this comment

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

I had the exact same question

Copy link
Contributor

Choose a reason for hiding this comment

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

its good to have naming which immediately tells the reader what does it mean. E.g. in this case - how its different from end_date. Examples: selected_end_date, formatted_end_date, localised_end_date

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is changed to end_date_input , thanks

const [day, setDay] = useState<number | null>(null);
const { server_time } = common;
Expand All @@ -53,31 +56,76 @@ const DayInput = ({
duration_min_max,
trade_types,
contract_type,
duration,
tick_data,
symbol,
barrier_1,
} = useTraderStore();
const trade_store = useTraderStore();
const { addSnackbar } = useSnackbar();

const new_values = {
duration_unit: 'd',
duration: day || '',
expiry_time: null,
duration: day || duration,
expiry_type: 'duration',
contract_type,
basis: 'stake',
amount: '5',
symbol,
};

const proposal_req = getProposalRequestObject({
new_values,
trade_store,
trade_type: Object.keys(trade_types)[0],
trade_type: contract_type === 'high_low' ? 'PUT' : Object.keys(trade_types)[0],
});
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we have to do this check for high_low?

Copy link
Contributor Author

@ahmadtaimoor-deriv ahmadtaimoor-deriv Oct 16, 2024

Choose a reason for hiding this comment

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

I added it initially for a specific usecase . its not reproducible now . I removed it , thanks

ahmadtaimoor-deriv marked this conversation as resolved.
Show resolved Hide resolved

const { data: response } = useDtraderQuery<ProposalResponse>(['proposal', JSON.stringify(day)], proposal_req, {
enabled: day !== null,
});
const { data: response } = useDtraderQuery<ProposalResponse>(
['proposal', JSON.stringify(day)],
(() => {
const request_payload = {
...proposal_req,
...{ symbol },
};

if (barrier_1) {
return {
...request_payload,
barrier: Number(Math.round(tick_data?.quote as number)),
};
Copy link
Contributor

Choose a reason for hiding this comment

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

is it needed to cast Number(..) after Math.round?
Math.round should already return number, unless there is some edge case here to support,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah I think Math.round will be enough . I'll change it , thanks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

}
return request_payload;
})(),
{
ahmadtaimoor-deriv marked this conversation as resolved.
Show resolved Hide resolved
enabled: trigger_date,
}
);

useEffect(() => {
if (response?.proposal?.date_expiry) {
setTempExpiryTime(
new Date((response?.proposal?.date_expiry as number) * 1000).toISOString().split('T')[1].substring(0, 8)
);
if (response) {
if (response?.error?.message && response?.error?.details?.field === 'duration') {
addSnackbar({
message: <Localize i18n_default_text={response?.error?.message} />,
status: 'fail',
hasCloseButton: true,
style: { marginBottom: '48px' },
});
setIsDisabled(true);
} else {
setIsDisabled(false);
}

if (response?.proposal?.date_expiry) {
setTempExpiryTime(
new Date((response?.proposal?.date_expiry as number) * 1000)
.toISOString()
.split('T')[1]
.substring(0, 8)
);
Copy link
Contributor

Choose a reason for hiding this comment

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

.split('T')[1].substring(0, 8) doesn't seems to be super safe, but I can't suggest something

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean you can ignore my comment

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 think it won't be an unsafe unless the response?.proposal?.date_expiry is something invalid like undefined . And so I already added check to see if response?.proposal?.date_expiry is valid.

}

invalidateDTraderCache(['proposal', JSON.stringify(day)]);
setTriggerDate(false);
}
}, [response, setTempExpiryTime]);

Expand Down Expand Up @@ -126,20 +174,26 @@ const DayInput = ({
setEndTime(adjusted_start_time);
}
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [end_date]);
}, [temp_end_date]);

let is_24_hours_contract = false;

const has_intraday_duration_unit = hasIntradayDurationUnit(duration_units_list);
is_24_hours_contract =
(!!start_date || toMoment(expiry_date || server_time).isSame(toMoment(server_time), 'day')) &&
has_intraday_duration_unit;
const parsedFormattedDate = new Date(Date.parse(`${formatted_date} 00:00:00`));

const isSameDate =
parsedFormattedDate.getFullYear() === server_time.year() &&
parsedFormattedDate.getMonth() === server_time.month() &&
parsedFormattedDate.getDate() === server_time.date();

is_24_hours_contract = (!!start_date || isSameDate) && has_intraday_duration_unit;

const handleDate = (date: Date) => {
const difference_in_time = date.getTime() - new Date().getTime();
const difference_in_days = Math.ceil(difference_in_time / (1000 * 3600 * 24));
setDay(Number(difference_in_days));
setEndDate(date);
setTempEndDate(date);
setTriggerDate(true);
};
return (
<div className='duration-container__days-input'>
Expand All @@ -162,8 +216,8 @@ const DayInput = ({
readOnly
textAlignment='center'
name='time'
value={`${(formatted_date === formatted_current_date ? end_time : temp_expiry_time) || '23:59:59'} GMT`}
disabled={formatted_date !== formatted_current_date || !is_24_hours_contract}
value={`${(is_24_hours_contract ? end_time : temp_expiry_time) || '23:59:59'} GMT`}
disabled={!is_24_hours_contract}
onClick={() => {
setOpenTimePicker(true);
}}
Expand All @@ -184,6 +238,8 @@ const DayInput = ({
onClose={() => {
setOpen(false);
setOpenTimePicker(false);
setIsDisabled(false);
setTempEndDate(end_date);
}}
position='left'
expandable={false}
Expand All @@ -206,7 +262,7 @@ const DayInput = ({
start_time,
duration_min_max
)}
end_date={end_date}
end_date={temp_end_date}
setEndDate={handleDate}
/>
)}
Expand All @@ -221,16 +277,26 @@ const DayInput = ({
<ActionSheet.Footer
alignment='vertical'
shouldCloseOnPrimaryButtonClick={false}
isPrimaryButtonDisabled={is_disabled}
primaryAction={{
content: <Localize i18n_default_text='Done' />,
onAction: () => {
setOpen(false);
setOpenTimePicker(false);
if (formatted_date !== formatted_current_date) {
setEndTime('');
}
if (timeToMinutes(adjusted_start_time) > timeToMinutes(end_time)) {
setEndTime(adjusted_start_time);
if (!is_disabled) {
setEndDate(temp_end_date);
setOpen(false);
setOpenTimePicker(false);
const end_date_temp = temp_end_date.toLocaleDateString('en-GB', {
day: 'numeric',
Copy link
Contributor

Choose a reason for hiding this comment

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

whats the flow / which one should be used when, given the:
end_date_temp, temp_end_date, end_date :D :D :D

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah yeah the names got confusing , let me rename thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So actually I added temp_end_date inside the action sheet as it saves the value temporarily inside the action sheet .
-> once we save the date , the value is moved to main duration component as end_date and temp_end_date is moved to default value.
-> if action sheet is closed without saving . temp_end_date is also changed to default value.

I changed the name to **end_date_input** now as it was making more sense rather then temp-.

Thanks

month: 'short',
year: 'numeric',
});

if (end_date_temp !== formatted_current_date) {
setEndTime('');
}
if (timeToMinutes(adjusted_start_time) > timeToMinutes(end_time)) {
setEndTime(adjusted_start_time);
}
}
},
}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,16 +85,10 @@ const Duration = observer(({ is_minimized }: TDurationProps) => {
expiry_type: 'duration',
});
}, 10);
const start_date = getDatePickerStartDate(duration_units_list, server_time, start_time, duration_min_max);

const are_dates_equal =
new Date(start_date).getDate() === end_date.getDate() &&
new Date(start_date).getMonth() === end_date.getMonth() &&
new Date(start_date).getFullYear() === end_date.getFullYear();
const start_date = getDatePickerStartDate(duration_units_list, server_time, start_time, duration_min_max);

if (!are_dates_equal) {
setEndDate(new Date(start_date));
}
setEndDate(new Date(start_date));

return () => clearTimeout(start_duration);
}, [symbol, contract_type, duration_min_max, duration_units_list]);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
import { WheelPickerContainer } from '@deriv-com/quill-ui';
import React, { useState, useEffect } from 'react';

type TimeOption = {
label: string;
value: number;
};

const HourPicker = ({
setWheelPickerValue,
selected_hour,
selected_time,
duration_min_max,
}: {
setWheelPickerValue: (index: number, value: string | number) => void;
selected_hour: number[];
selected_time: number[];
duration_min_max: Record<string, { min: number; max: number }>;
}) => {
const [hours, setHours] = useState<TimeOption[]>([]);
const [minutes, setMinutes] = useState<TimeOption[]>([]);

useEffect(() => {
const min_seconds = Math.max(duration_min_max.intraday.min, 3600);
const max_seconds = duration_min_max.intraday.max;

const min_hours = Math.max(1, Math.ceil(min_seconds / 3600));
const max_hours = Math.floor(max_seconds / 3600);

const new_hours = Array.from({ length: max_hours - min_hours + 1 }, (_, i) => ({
label: `${i + min_hours} h`,
value: i + min_hours,
}));
setHours(new_hours);

update_minutes(selected_hour[0] || min_hours, min_seconds, max_seconds);
}, [duration_min_max, selected_hour]);

const update_minutes = (selected_hour: number, min_seconds: number, max_seconds: number) => {
let min_minutes = 0;
let max_minutes = 59;

if (selected_hour === Math.ceil(min_seconds / 3600)) {
min_minutes = Math.ceil((min_seconds % 3600) / 60);
}

if (selected_hour === Math.floor(max_seconds / 3600)) {
max_minutes = Math.floor((max_seconds % 3600) / 60);
}

if (min_minutes > 0 && selected_hour * 3600 >= min_seconds) {
min_minutes = 0;
}

const new_minutes = Array.from({ length: max_minutes - min_minutes + 1 }, (_, i) => ({
label: `${i + min_minutes} min`,
value: i + min_minutes,
}));
setMinutes(new_minutes);
};

const handle_value_change = (index: number, value: string | number) => {
setWheelPickerValue(index, value);
if (index === 0) {
update_minutes(Number(value), duration_min_max.intraday.min, duration_min_max.intraday.max);
}
};

const getDefaultValue = (options: TimeOption[], selected_value: number) => {
const option = options.find(opt => opt.value === selected_value);
return option ? option.label : options[0]?.label || '1 h';
};

return (
<WheelPickerContainer
data={[hours, minutes]}
defaultValue={[getDefaultValue(hours, selected_hour[0]), getDefaultValue(minutes, selected_time[1])]}
containerHeight={'268px'}
ahmadtaimoor-deriv marked this conversation as resolved.
Show resolved Hide resolved
inputValues={[selected_hour[0] || hours[0]?.value, selected_time[1] || minutes[0]?.value]}
setInputValues={handle_value_change}
/>
);
};

export default HourPicker;
Loading
Loading