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

fix(data-grid): improve ts typings and react proptypes #3261

Merged
merged 3 commits into from Jun 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/short-gifts-glow.md
@@ -0,0 +1,7 @@
---
'@twilio-paste/data-grid': patch
'@twilio-paste/table': patch
'@twilio-paste/core': patch
---

[Table, DataGrid] Typescript type improvements for Tr, Th, and Td elements
Expand Up @@ -7,7 +7,7 @@ import {Tr} from './table/Tr';
import {DataGridContext} from './DataGridContext';

export interface DataGridRowProps {
children?: React.ReactNode;
children: NonNullable<React.ReactNode>;
selected?: boolean;
element?: BoxElementProps['element'];
}
Expand Down
10 changes: 4 additions & 6 deletions packages/paste-core/components/data-grid/src/table/Td.tsx
@@ -1,13 +1,11 @@
import * as React from 'react';
import PropTypes from 'prop-types';
import {Box, safelySpreadBoxProps} from '@twilio-paste/box';
import type {BoxElementProps} from '@twilio-paste/box';
import type {TextAlign} from '@twilio-paste/style-props';
import {TdPropTypes} from '@twilio-paste/table';
import type {TdProps as TableTdProps} from '@twilio-paste/table';

export interface TdProps {
textAlign?: TextAlign;
export interface TdProps extends TableTdProps {
onClick?: React.MouseEventHandler;
element?: BoxElementProps['element'];
}

export const Td = React.forwardRef<HTMLTableCellElement, TdProps>(
Expand Down Expand Up @@ -47,6 +45,6 @@ export const Td = React.forwardRef<HTMLTableCellElement, TdProps>(

Td.displayName = 'Td';
Td.propTypes = {
...TdPropTypes,
onClick: PropTypes.func,
element: PropTypes.string,
};
14 changes: 4 additions & 10 deletions packages/paste-core/components/data-grid/src/table/Th.tsx
@@ -1,15 +1,11 @@
import * as React from 'react';
import PropTypes from 'prop-types';
import {Box, safelySpreadBoxProps} from '@twilio-paste/box';
import type {BoxElementProps} from '@twilio-paste/box';
import type {TextAlign} from '@twilio-paste/style-props';
import {ThPropTypes} from '@twilio-paste/table';
import type {ThProps as TableThProps} from '@twilio-paste/table';

export interface ThProps {
textAlign?: TextAlign;
width?: string;
export interface ThProps extends TableThProps {
onClick?: React.MouseEventHandler;
element?: BoxElementProps['element'];
colSpan?: number;
}

export const Th = React.forwardRef<HTMLTableCellElement, ThProps>(
Expand Down Expand Up @@ -50,8 +46,6 @@ export const Th = React.forwardRef<HTMLTableCellElement, ThProps>(

Th.displayName = 'Th';
Th.propTypes = {
...ThPropTypes,
onClick: PropTypes.func,
width: PropTypes.string,
element: PropTypes.string,
colSpan: PropTypes.number,
};
9 changes: 5 additions & 4 deletions packages/paste-core/components/data-grid/src/table/Tr.tsx
Expand Up @@ -2,13 +2,13 @@ import * as React from 'react';
import PropTypes from 'prop-types';
import {styled, css} from '@twilio-paste/styling-library';
import {Box, safelySpreadBoxProps} from '@twilio-paste/box';
import type {BoxElementProps} from '@twilio-paste/box';
import {TrPropTypes} from '@twilio-paste/table';
import type {TrProps as TableTrProps} from '@twilio-paste/table';

export interface TrProps {
export interface TrProps extends TableTrProps {
role: string;
striped: boolean;
selected?: boolean;
element?: BoxElementProps['element'];
}

const StyledTr = styled.tr<TrProps>(
Expand Down Expand Up @@ -43,7 +43,8 @@ export const Tr = React.forwardRef<HTMLTableRowElement, TrProps>(

Tr.displayName = 'Tr';
Tr.propTypes = {
...TrPropTypes,
role: PropTypes.string.isRequired,
selected: PropTypes.bool,
element: PropTypes.string,
striped: PropTypes.bool.isRequired,
};
14 changes: 4 additions & 10 deletions packages/paste-core/components/table/src/proptypes.ts
Expand Up @@ -18,11 +18,8 @@ export const THeadPropTypes = {
children: PropTypes.node.isRequired,
element: PropTypes.string,
};

export const TBodyPropTypes = {
children: PropTypes.node.isRequired,
element: PropTypes.string,
};
export const TBodyPropTypes = THeadPropTypes;
export const TFootPropTypes = THeadPropTypes;

export const TrPropTypes = {
children: PropTypes.node.isRequired,
Expand All @@ -35,15 +32,12 @@ export const ThPropTypes = {
element: PropTypes.string,
textAlign: PropTypes.oneOf(Object.values(TableAlignmentObject)),
width: isWidthTokenProp,
colSpan: PropTypes.number,
};

export const TdPropTypes = {
colSpan: PropTypes.number,
children: PropTypes.node,
element: PropTypes.string,
textAlign: PropTypes.oneOf(Object.values(TableAlignmentObject)),
};

export const TFootPropTypes = {
children: PropTypes.node.isRequired,
element: PropTypes.string,
};
3 changes: 1 addition & 2 deletions packages/paste-core/components/table/src/types.ts
Expand Up @@ -62,9 +62,8 @@ export interface TrProps extends React.TableHTMLAttributes<HTMLTableRowElement>
children: NonNullable<React.ReactNode>;
element?: BoxProps['element'];
verticalAlign?: TableVerticalAlignmentOptions;
isLastRow?: boolean;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Wasn't being used, does nothing.

Copy link
Collaborator

Choose a reason for hiding this comment

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

question: is there a chance it was being used by someone even if it was doing nothing? if so would we want to think twice about removing it to avoid giving someone a new ts error? sidenote: why did we add it in the first place 🤔

}
export interface ThProps extends React.ThHTMLAttributes<HTMLTableHeaderCellElement> {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

HTMLTableHeaderCellElement was deprecated in favor of HTMLTableCellElement

export interface ThProps extends React.ThHTMLAttributes<HTMLTableCellElement> {
children?: React.ReactNode;
element?: BoxProps['element'];
textAlign?: TableAlignmentOptions;
Expand Down