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.

Sign up for GitHub

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

Jump to bottom

Sergei / chore: migrate open-positions to tsx #40

Merged
merged 10 commits into from
Jan 26, 2023
Merged

Sergei / chore: migrate open-positions to tsx #40

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
cf05b3e
chore: convert open-positions to tsx
sergei-deriv Jan 18, 2023
9673fe7
fix: add React.PropsWithChildren to tabs.tsx
sergei-deriv Jan 18, 2023
7b957ef
fix: change Tabs type
sergei-deriv Jan 18, 2023
0814e8d
fix: add/change some types and fix some undefined values
sergei-deriv Jan 19, 2023
2285054
refactor: use Return Early pattern for contract-card-sell component
sergei-deriv Jan 19, 2023
e4bda7b
chore: delete commented lines
sergei-deriv Jan 24, 2023
5c13c20
chore: add moment as dev dependency
sergei-deriv Jan 25, 2023
2f5787d
chore: commit to check circleCI
sergei-deriv Jan 26, 2023
7158294
fix: change back tabs.tsx
sergei-deriv Jan 26, 2023
993b611
fix: change React.Fragment to null
sergei-deriv Jan 26, 2023
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
36 changes: 18 additions & 18 deletions packages/components/src/components/contract-card/contract-card-items/contract-card-sell.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,24 +14,24 @@ const ContractCardSell = ({ contract_info, getCardLabels, is_sell_requested, onC
ev.preventDefault();
};

return (
should_show_sell && (
<React.Fragment>
{is_valid_to_sell ? (
<Button
className={classNames('dc-btn--sell', {
'dc-btn--loading': is_sell_requested,
})}
is_disabled={is_sell_requested}
text={getCardLabels().SELL}
onClick={onClick}
secondary
/>
) : (
<div className='dc-contract-card__no-resale-msg'>{getCardLabels().RESALE_NOT_OFFERED}</div>
)}
</React.Fragment>
)
return should_show_sell ? (
Copy link

Choose a reason for hiding this comment

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

@sergei-deriv Not related to your PR, But since you have changed the condition I suggest you change it to the Return Early pattern.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@sergei-deriv Not related to your PR, But since you have changed the condition I suggest you change it to the Return Early pattern.

I did it because with syntax "return should_show_sell && ( JSX )" this component returned JSX of False. But with my refactored code "return should_show_sell ? ( JSX ) : (JSX)" this component always returns JSX.

Copy link

Choose a reason for hiding this comment

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

@sergei-deriv You can use the Return Early pattern for better readability, It's still the same.

if (!should_show_sell) return <></>;
if (!is_valid_to_sell) return <div...
return <Button...

It's just a suggestion tho 😅 Feel free to ignore it 🙇🏻

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@sergei-deriv You can use the Return Early pattern for better readability, It's still the same.

if (!should_show_sell) return <></>;
if (!is_valid_to_sell) return <div...
return <Button...

It's just a suggestion tho 😅 Feel free to ignore it 🙇🏻

It's really good advice, I fully agree with you. I've updated my commit.

<React.Fragment>
{is_valid_to_sell ? (
<Button
className={classNames('dc-btn--sell', {
'dc-btn--loading': is_sell_requested,
})}
is_disabled={is_sell_requested}
text={getCardLabels().SELL}
onClick={onClick}
secondary
/>
) : (
<div className='dc-contract-card__no-resale-msg'>{getCardLabels().RESALE_NOT_OFFERED}</div>
)}
</React.Fragment>
) : (
<React.Fragment />
);
};

Expand Down
2 changes: 1 addition & 1 deletion packages/components/src/components/progress-bar/progress-bar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import React from 'react';
import Text from '../text';

// Todo: Create a utility type for getting a range of float number. ex: FloatRange<0, 1>
Copy link

Choose a reason for hiding this comment

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

@sergei-deriv does this PR also implies that we address the TODO comment? 🙂 if yes, we can make a TFloatRange type according to an example in https://catchts.com/range-numbers

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@sergei-deriv does this PR also implies that we address the TODO comment? 🙂 if yes, we can make a TFloatRange type according to an example in https://catchts.com/range-numbers

I'm not sure, honestly :) But if you think I have to do this, I'll try. But for my PR I've got this type like this:
type TRangeFloatZeroToOne = React.ComponentProps['value'];

Copy link

Choose a reason for hiding this comment

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

I think it’s out of this PR scope to handle the todo, But it will be nice if we add the generic @maryia-deriv suggested to utils.d.ts 👍🏻 Maybe in a separate card 🤔

type TRangeFloatZeroToOne = 0 | 0.1 | 0.2 | 0.3 | 0.4 | 0.5 | 0.6 | 0.7 | 0.8 | 0.9 | 1;
export type TRangeFloatZeroToOne = 0 | 0.1 | 0.2 | 0.3 | 0.4 | 0.5 | 0.6 | 0.7 | 0.8 | 0.9 | 1;
Copy link

Choose a reason for hiding this comment

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

@sergei-deriv No need to export, Please use React.ComponentProps generic.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@sergei-deriv No need to export, Please use React.ComponentProps generic.

I've added this type in 'open-positions' component:
type TRangeFloatZeroToOne = React.ComponentProps['value'];


type TProgressBarProps = {
className?: string;
Expand Down
48 changes: 24 additions & 24 deletions packages/components/src/components/tabs/tabs.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,30 +5,30 @@ import Tab from './tab';
import { useConstructor } from '../../hooks';
import ThemedScrollbars from '../themed-scrollbars/themed-scrollbars';

type TTabsProps = RouteComponentProps & {
active_icon_color: string;
active_index?: number;
background_color: string;
bottom: boolean;
center: boolean;
children: React.ReactElement & React.ReactElement[];
className?: string;
fit_content: boolean;
has_active_line?: boolean;
has_bottom_line?: boolean;
header_fit_content: boolean;
history: History;
icon_color: string;
icon_size: number;
is_100vw: boolean;
is_full_width: boolean;
is_overflow_hidden: boolean;
is_scrollable: boolean;
onTabItemClick: (active_tab_index: number) => void;
should_update_hash: boolean;
single_tab_has_no_label: boolean;
top: boolean;
};
type TTabsProps = RouteComponentProps &
React.PropsWithChildren<{
active_icon_color: string;
active_index?: number;
background_color: string;
bottom: boolean;
center: boolean;
className?: string;
fit_content: boolean;
has_active_line?: boolean;
has_bottom_line?: boolean;
header_fit_content: boolean;
history: History;
icon_color: string;
icon_size: number;
is_100vw: boolean;
is_full_width: boolean;
is_overflow_hidden: boolean;
is_scrollable: boolean;
onTabItemClick: (active_tab_index: number) => void;
should_update_hash: boolean;
single_tab_has_no_label: boolean;
top: boolean;
}>;

const Tabs = ({
active_icon_color,
Expand Down
8 changes: 4 additions & 4 deletions packages/reports/src/Containers/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import OpenPositions from './open-positions.jsx';
import ProfitTable from './profit-table.jsx';
import Statement from './statement.jsx';
import Reports from './reports.jsx';
import OpenPositions from './open-positions';
import ProfitTable from './profit-table';
import Statement from './statement';
import Reports from './reports';

export default {
OpenPositions,
Expand Down
Loading