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

Yauheni/76933/table component ts migration #73

Merged
Show file tree
Hide file tree
Changes from 9 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
@@ -1,4 +1,4 @@
import Table from './table.jsx';
import Table from './table';
import './table.scss';

export default Table;
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
import React from 'react';

const Body = ({ children, ...otherProps }) => (
type TBody = {
className?: string;
};

const Body = ({ children, ...otherProps }: React.PropsWithChildren<TBody>) => (
<div role='rowgroup' {...otherProps}>
{children}
</div>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
import React from 'react';
import classNames from 'classnames';
import PropTypes from 'prop-types';

const Cell = ({ children, align = 'left', className, fixed }) => (
export type TCell = {
align: 'left' | 'right';
className: string;
fixed: boolean;
};

const Cell = ({ children, align = 'left', className, fixed }: React.PropsWithChildren<Partial<TCell>>) => (
<div
role='cell'
className={classNames('dc-table__cell', className, {
Expand All @@ -14,11 +19,4 @@ const Cell = ({ children, align = 'left', className, fixed }) => (
</div>
);

Cell.propTypes = {
align: PropTypes.oneOf(['left', 'right']),
children: PropTypes.node,
className: PropTypes.string,
fixed: PropTypes.bool,
};

export default Cell;
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import React from 'react';
import classNames from 'classnames';
import PropTypes from 'prop-types';
import { TCell } from './table-cell';

const Head = ({ children, align, className, fixed }) => (
const Head = ({ children, align, className, fixed }: React.PropsWithChildren<Partial<TCell>>) => (
<div
role='columnheader'
className={classNames('dc-table__head', className, {
Expand All @@ -14,11 +14,4 @@ const Head = ({ children, align, className, fixed }) => (
</div>
);

Head.propTypes = {
align: PropTypes.oneOf(['left', 'right']),
children: PropTypes.node,
className: PropTypes.string,
fixed: PropTypes.bool,
};

export default Head;
Original file line number Diff line number Diff line change
@@ -1,16 +1,15 @@
import React from 'react';
import classNames from 'classnames';
import PropTypes from 'prop-types';

const Header = ({ children, className }) => (
type THeader = {
children: React.ReactNode;

Choose a reason for hiding this comment

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

is there a reason that you didn't use React.PropsWithChildren here?

Copy link
Author

Choose a reason for hiding this comment

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

because children are required here. and children in PropsWithChildren are optional

Choose a reason for hiding this comment

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

Got it.

className?: string;
};

const Header = ({ children, className }: THeader) => (

Choose a reason for hiding this comment

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

Suggested change
const Header = ({ children, className }: THeader) => (
const Header = ({ children, className }: React.PropsWithChildren<THeader>) => (
type THeader = {
    className?: string;
};

Copy link
Author

Choose a reason for hiding this comment

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

thanks for this comment. in other cases it would be great, but here children can’t be optional. in PropsWithChildren children is optional.

<div role='rowgroup' className={classNames('dc-table__header', className)}>
{children}
</div>
);

Header.propTypes = {
children: PropTypes.node.isRequired,
className: PropTypes.string,
};

export default Header;
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
import React from 'react';
import classNames from 'classnames';
import PropTypes from 'prop-types';

const Row = ({ children, className, has_hover }) => {
type TRow = {
children: React.ReactNode;
className?: string;
has_hover?: boolean;
};

const Row = ({ children, className, has_hover }: TRow) => {

Choose a reason for hiding this comment

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

Suggested change
const Row = ({ children, className, has_hover }: TRow) => {
const Row = ({ children, className, has_hover }: React.PropsWithChildren<TRow>) => {
type TRow = {
    className?: string;
    has_hover?: boolean;
};

@yauheni-kryzhyk-deriv please also address an issue in packages/p2p/src/components/orders/order-table/order-table-row.jsx where we pass an additional style prop to this component but we don't use it inside it. 🙂

Copy link
Author

Choose a reason for hiding this comment

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

same is here: children are required

return (
<div
role='row'
Expand All @@ -15,10 +20,4 @@ const Row = ({ children, className, has_hover }) => {
);
};

Row.propTypes = {
children: PropTypes.node.isRequired,
className: PropTypes.string,
has_hover: PropTypes.bool,
};

export default Row;
Original file line number Diff line number Diff line change
@@ -1,14 +1,27 @@
import React from 'react';
import classNames from 'classnames';
import Head from './table-head.jsx';
import Header from './table-header.jsx';
import Body from './table-body.jsx';
import Row from './table-row.jsx';
import Cell from './table-cell.jsx';
import Head from './table-head';
import Header from './table-header';
import Body from './table-body';
import Row from './table-row';
import Cell from './table-cell';
import ThemedScrollbars from '../themed-scrollbars/themed-scrollbars';

type TTable = {
className: string;
fixed: boolean;
scroll_width: string;
scroll_height: string;
};

// TODO: update the <Table /> component to fit with the DataTable in Trader
const Table = ({ className, fixed, children, scroll_width, scroll_height }) => (
const Table = ({
className,
fixed,
children,
scroll_width,
scroll_height,
}: React.PropsWithChildren<Partial<TTable>>) => (
<div
role='table'
className={classNames('dc-table', className, {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ type TThemedScrollbars = {
is_only_horizontal?: boolean;
is_only_horizontal_overlay?: boolean;
is_scrollbar_hidden?: boolean;
onScroll: UIEventHandler<HTMLDivElement>;
refSetter: RefObject<HTMLDivElement>;
onScroll?: UIEventHandler<HTMLDivElement>;
refSetter?: RefObject<HTMLDivElement> | null;
style?: React.CSSProperties;
width?: string;
};
Expand All @@ -28,13 +28,13 @@ const ThemedScrollbars = ({
is_only_horizontal_overlay,
is_scrollbar_hidden,
onScroll,
refSetter,
refSetter = null,
style = {},
width,
}: React.PropsWithChildren<TThemedScrollbars>) => {
const [hoverRef, isHovered] = useHover<HTMLDivElement>(refSetter, false);

if (is_bypassed) return children;
if (is_bypassed) return children as JSX.Element;
return (
<div
data-testid='dt_themed_scrollbars'
Expand Down
28 changes: 12 additions & 16 deletions packages/components/stories/table/README.md
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
# Table component

Use this component to have a basic table.

## Usage

```jsx
import { Table } from 'deriv-components';

const DummyComponent = (props) => (
const DummyComponent = props => (
<Table>
<Table.Header>
<Table.Row className='table-storybook-row'>
Expand All @@ -25,29 +26,24 @@ const DummyComponent = (props) => (
))}
</Table.Body>
</Table>
)
);
```

## Props

| Name | Type | Default | Description |
|--------------------------|------------------------|--------------------|--------------------------------------------------------------------------------------------------------------------------|
| fixed | {boolean} | null | Set it to 'true' to have customed scrollbar for the table |
| scroll\_width | {string \| number} | 100% | If 'fixed' is 'true' you can set fixed width of the table |
| scroll\_height | {string \| number} | null | If 'fixed' is 'true' you can set fixed height of the table |

| Name | Type | Default | Description |
| ------------- | ------------------ | ------- | ---------------------------------------------------------- |
| fixed | {boolean} | null | Set it to 'true' to have customed scrollbar for the table |
| scroll_width | {string \| number} | 100% | If 'fixed' is 'true' you can set fixed width of the table |
| scroll_height | {string \| number} | null | If 'fixed' is 'true' you can set fixed height of the table |

## Full example:

```jsx
import { Table } from 'deriv-components';

const DummyComponent = (props) => (
<Table
fixed
scroll_width={400}
scroll_height={400}
>
const DummyComponent = props => (
<Table fixed scroll_width='400' scroll_height='400'>
<Table.Header>
<Table.Row className='table-storybook-row'>
{props.data.header.map((field, index) => (
Expand All @@ -65,5 +61,5 @@ const DummyComponent = (props) => (
))}
</Table.Body>
</Table>
)
);
```
2 changes: 1 addition & 1 deletion packages/components/stories/table/stories/fixed-size.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import sample_data from '../sample-data.json';

const FixedSize = () => (
<Wrapper is_dark={boolean('Dark Theme', false)}>
<Table fixed scroll_width={400} scroll_height={400}>
<Table fixed scroll_width='400' scroll_height='400'>

Choose a reason for hiding this comment

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

@yauheni-kryzhyk-deriv please also update packages/components/stories/table/README.md lines 48,49 🙂

<Table.Header>
<Table.Row className='table-storybook-row'>
{sample_data.fields.map((field, index) => (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ const Title = ({ send_amount, currency, order_purchase_datetime, order_type }) =
);
};

const OrderRow = ({ style, row: order }) => {

Choose a reason for hiding this comment

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

Just for my understanding, we don't need this property, am I right?

Copy link
Author

Choose a reason for hiding this comment

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

yes, it seems we don’t use it inside table.row

const OrderRow = ({ row: order }) => {
const getTimeLeft = time => {
const distance = ServerTime.getDistanceToServerTime(time);
return {
Expand Down Expand Up @@ -175,7 +175,6 @@ const OrderRow = ({ style, row: order }) => {
</DesktopWrapper>
<MobileWrapper>
<Table.Row
style={style}
className={classNames('orders__mobile', {
'orders__mobile--attention': !isOrderSeen(id),
})}
Expand Down Expand Up @@ -257,7 +256,6 @@ const OrderRow = ({ style, row: order }) => {

OrderRow.propTypes = {
order: PropTypes.object,
style: PropTypes.object,
row: PropTypes.object,
server_time: PropTypes.object,
};
Expand Down
2 changes: 1 addition & 1 deletion packages/p2p/src/stores/my-profile-store.js
Original file line number Diff line number Diff line change
Expand Up @@ -524,7 +524,7 @@ export default class MyProfileStore extends BaseStore {
this.setIsConfirmDeleteModalOpen(true);
}
}

Choose a reason for hiding this comment

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

@yauheni-kryzhyk-deriv do we need to change it in this PR?😊

Copy link
Author

Choose a reason for hiding this comment

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

Don’t understand this issue, when I delete this space it shows me this deleting like my changes. If I add it, it shows me, that it is also my changes. will leave it with space as it is the same in the code

onSubmit() {
const { general_store } = this.root_store;

Expand Down