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

chore(react-integration): add tables to demos and write unit tests #2685

Merged
merged 13 commits into from Sep 13, 2019

Conversation

@jenny-s51
Copy link
Contributor

jenny-s51 commented Aug 12, 2019

What: in relation to #2673

@jenny-s51 jenny-s51 requested a review from priley86 Aug 12, 2019

import DemoSortableTable from '@patternfly/react-table/src/components/Table/demo/DemoSortableTable';

export class TableCompoundExpandableDemo extends React.Component<{}, { columns: any, rows: any[] }> {

This comment has been minimized.

Copy link
@jenny-s51

jenny-s51 Aug 12, 2019

Author Contributor

Converting from JS to TSX. Receiving error: "You may need an appropriate loader to handle this file type."

This comment has been minimized.

Copy link
@priley86

priley86 Aug 12, 2019

Member

Yes, we should convert DemoSortableTable to TSX locally in this example. Good catch!

caption="Actions Table"
cells={columns}
rows={rows}
actionResolver={() => null as (IAction | ISeparator)[]}

This comment has been minimized.

Copy link
@jenny-s51

jenny-s51 Aug 12, 2019

Author Contributor

Changed to this function declaration from actionResolver={this.actionResolver}. Was getting a type error here before... not sure if this will work

This comment has been minimized.

Copy link
@priley86

priley86 Aug 12, 2019

Member

yes... we can try to cast these callbacks from our interface correctly now.. so this becomes something like this:

import React from 'react';
import {
  Table,
  TableHeader,
  TableBody,
  sortable,
  SortByDirection,
  headerCol,
  TableVariant,
  expandable,
  cellWidth,
  IRow,
  IRowData,
  IExtra,
  IActionsResolver,
  IAction,
  ISeparator
} from '@patternfly/react-table';

export class TableActionsDemo extends React.Component<{}, { columns: any, rows: IRow[] }> {
  constructor(props) {
    super(props);
    this.state = {
      columns: [
        { title: 'Repositories', cellTransforms: [headerCol()] },
        'Branches',
        { title: 'Pull requests' },
        'Workspaces',
        'Last Commit'
      ],
      rows: [
        {
          cells: ['one', 'two', 'a', 'four', 'five'],
          type: 'green'
        },
        {
          cells: ['a', 'two', 'k', 'four', 'five']
        },
        {
          cells: ['p', 'two', 'b', 'four', 'five'],
          type: 'blue'
        },
        {
          cells: ['5', '2', 'b', 'four', 'five']
        }
      ]
    };
  }

  actionResolver(rowData: IRowData, { rowIndex }: IExtra) {
    if (rowIndex === 1) {
      return null;
    }

    const thirdAction =
      rowData.type === 'blue'
        ? [
            {
              isSeparator: true
            } as ISeparator,
            {
              title: 'Third action',
              // tslint:disable-next-line:no-shadowed-variable
              onClick: (event, rowId, rowData, extra) =>
                // tslint:disable-next-line:no-console
                console.log(`clicked on Third action, on row ${rowId} of type ${rowData.type}`)
            } as IAction
          ]
        : [];

    return [
      {
        title: 'Some action',
        // tslint:disable-next-line:no-shadowed-variable
        onClick: (event, rowId, rowData, extra) =>
          // tslint:disable-next-line:no-console
          console.log(`clicked on Some action, on row ${rowId} of type ${rowData.type}`)
      } as IAction,
      {
        title: <div>Another action</div>,
        // tslint:disable-next-line:no-shadowed-variable
        onClick: (event, rowId, rowData, extra) =>
          // tslint:disable-next-line:no-console
          console.log(`clicked on Another action, on row ${rowId} of type ${rowData.type}`)
      } as IAction,
      ...thirdAction
    ];
  }

  areActionsDisabled(rowData, { rowIndex }) {
    return rowIndex === 3;
  }

  render() {
    const { columns, rows } = this.state;
    return (
      <Table
        caption="Actions Table"
        cells={columns}
        rows={rows}
        actionResolver={this.actionResolver}
        areActionsDisabled={this.areActionsDisabled}
      >
        <TableHeader />
        <TableBody />
      </Table>
    );
  }
}

This comment has been minimized.

Copy link
@jenny-s51

jenny-s51 Aug 13, 2019

Author Contributor

Thank you Patrick!! This is very helpful, I can now see where and why you added those specific type assertions 🙂

@priley86

This comment has been minimized.

Copy link
Member

priley86 commented Aug 12, 2019

really nice work! Nice to see this 😄 .

@jenny-s51 jenny-s51 force-pushed the jenny-s51:tables-integration branch from eb6ff16 to 4782f1e Aug 14, 2019
@patternfly-build

This comment has been minimized.

Copy link
Contributor

patternfly-build commented Aug 14, 2019

PatternFly-React preview: https://patternfly-react-pr-2685.surge.sh

}

export interface ICell {
title?: string;
transforms?: ((...args: any) => any)[];

This comment has been minimized.

Copy link
@priley86

priley86 Aug 20, 2019

Member

I pushed a commit experimenting w/ this just now:
priley86@dd3b444

I'm seeing that something like this might work. I would just make sure that all use cases are covered in your integration tests though.

export interface ICell {
  title?: string;
  transforms?: ((label?: IFormatterValueType, rowData?: IRowData, columnIndex?: number, column?: IColumn, property?: string, rowIndex?: number, rowKey?: RowKeyType ) => { className: string; 'aria-sort': string; children: React.ReactNode; })[];
  cellTransforms?: ((label?: IFormatterValueType, rowData?: IRowData, columnIndex?: number, column?: IColumn, property?: string, rowIndex?: number, rowKey?: RowKeyType ) => { className: string; 'aria-sort': string; children: React.ReactNode; })[];
  columnTransforms?: ((label?: IFormatterValueType, rowData?: IRowData, columnIndex?: number, column?: IColumn, property?: string, rowIndex?: number, rowKey?: RowKeyType ) => { className: string; 'aria-sort': string; children: React.ReactNode; })[];
  formatters?: ((data?: IFormatterValueType, rowData?: IRowData, columnIndex?: number, column?: IColumn, property?: string, rowIndex?: number, rowKey?: RowKeyType ) => formatterValueType)[];
  cellFormatters?: ((data?: IFormatterValueType, rowData?: IRowData, columnIndex?: number, column?: IColumn, property?: string, rowIndex?: number, rowKey?: RowKeyType ) => formatterValueType)[];
@@ -0,0 +1,93 @@
import React from 'react';

This comment has been minimized.

Copy link
@jschuler

jschuler Aug 20, 2019

Collaborator

TS way would be import * as React from 'react';

IRow,
} from '@patternfly/react-table';

export class TableCollapsibleDemo extends React.Component<{}, { columns: any, rows: IRow[] }> {

This comment has been minimized.

Copy link
@jschuler

jschuler Aug 20, 2019

Collaborator

Should import TableProps and set that as first argument in React.Component<>

This comment has been minimized.

Copy link
@jschuler

jschuler Aug 21, 2019

Collaborator

Oops no, nevermind :) Not needed here

This comment has been minimized.

Copy link
@jenny-s51

jenny-s51 Aug 21, 2019

Author Contributor

@jschuler I just pushed a new commit that sets TableProps as the first argument there -- did you mean for just this file or for all demos? I can change them all back to use the same syntax as before

This comment has been minimized.

Copy link
@jenny-s51

jenny-s51 Aug 21, 2019

Author Contributor

It looks like the change you suggested also works @jschuler :)

} from '@patternfly/react-table';

export class TableCollapsibleDemo extends React.Component<{}, { columns: any, rows: IRow[] }> {
constructor(props) {

This comment has been minimized.

Copy link
@jschuler

jschuler Aug 20, 2019

Collaborator

Set props to be of type TableProps

This comment has been minimized.

Copy link
@priley86

priley86 Aug 20, 2019

Member

yes... this should help find any more issues 👍

@jenny-s51 jenny-s51 force-pushed the jenny-s51:tables-integration branch 2 times, most recently from 953d54d to 64e20a6 Aug 21, 2019
@jenny-s51 jenny-s51 requested review from jschuler and priley86 Aug 21, 2019
@jenny-s51 jenny-s51 changed the title WIP(adding tables to react-integration) chore(react-integration): add tables to demos and write unit tests Aug 21, 2019
@jenny-s51 jenny-s51 requested review from dlabaj and tlabaj Sep 6, 2019
@jenny-s51 jenny-s51 force-pushed the jenny-s51:tables-integration branch 2 times, most recently from 4b8ccea to 35323e0 Sep 6, 2019
jenny-s51 added 3 commits Sep 6, 2019
@dlabaj
dlabaj approved these changes Sep 9, 2019
@priley86

This comment has been minimized.

Copy link
Member

priley86 commented Sep 13, 2019

tested this downstream - no issues here. looks great @jenny-s51 !

@priley86 priley86 requested a review from seanforyou23 Sep 13, 2019
@@ -37,6 +37,5 @@ export const BodyWrapper: React.FunctionComponent<BodyWrapperProps> = ({
</React.Fragment>
);
}

This comment has been minimized.

Copy link
@redallen

redallen Sep 13, 2019

Contributor

I can tell you looked at this file :)

rowIndex?: number,
rowKey?: RowKeyType ) => { className: string; 'aria-sort': string; children: React.ReactNode; })[];

export type IFormatters = ((

This comment has been minimized.

Copy link
@redallen

redallen Sep 13, 2019

Contributor

Thanks for the added interface.

@redallen redallen merged commit 71a8dab into patternfly:master Sep 13, 2019
8 checks passed
8 checks passed
ci/circleci: build Your tests passed on CircleCI!
Details
ci/circleci: build_integration Your tests passed on CircleCI!
Details
ci/circleci: build_pf3_docs Your tests passed on CircleCI!
Details
ci/circleci: build_pf4_docs Your tests passed on CircleCI!
Details
ci/circleci: lint Your tests passed on CircleCI!
Details
ci/circleci: test_jest_other Your tests passed on CircleCI!
Details
ci/circleci: test_jest_pf4 Your tests passed on CircleCI!
Details
ci/circleci: upload_docs Your tests passed on CircleCI!
Details
@patternfly-build

This comment has been minimized.

Copy link
Contributor

patternfly-build commented Sep 13, 2019

Your changes have been released in:

  • @patternfly/react-docs@4.13.3
  • @patternfly/react-inline-edit-extension@2.11.37
  • demo-app-ts@2.24.11
  • @patternfly/react-integration@2.24.2
  • @patternfly/react-table@2.20.17
  • @patternfly/react-virtualized-extension@1.2.25

Thanks for your contribution! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.