feat(tables): adds reactabular and patternfly tables examples #143

priley86 · 2017-12-23T18:39:47Z

What:
Ho, ho, ho PatternFly React! How about some Data Tables from Santa this year?? 🎅 🎄 🎁

This PR starts the data table contribution w/ Reactabular.

Reactabular is a very powerful library, one that I've barely scratched the surface of here...

Some initial things i've showcased here:

sorting w/ sortabular
custom table elements for header cells/table cells. guide
header props/formatters and cell props/formatters
customer resolvers for resolving rows

As one can see, this is an extremely flexible React table...

Some things I'd like to do soon...

Add pagination / pagination components
Add row selection example
Add column visibility filters example (just sets the hidden attribute on columns)
Add empty state abstractions when rows are empty or all data has been filtered
Add any a11y attributes we'd like...
Some general cleanup/extendability... (this was put together quite quickly)
maybe even ask @bebraw for some suggestions? he's on gitter a lot it seems ;)

Link to Storybook

Additional issues:
I am off on PTO until 1/3/2018. Maybe someone can review this in the meantime and provide any feedback?

cc: @jeff-phillips-18 @ohadlevy @jgiardino @yaacov @amirfefer @jtomasek

Merry Christmas!

serenamarie125 · 2017-12-23T19:09:29Z

Nice gift @priley86 🎁 🎄
🤶🏼 Merry Christmas

bebraw · 2017-12-26T08:31:54Z

Great to see Reactabular in action. Feel free to PR something to my README after merge so people find patternfly. 👍

priley86 · 2017-12-27T11:11:51Z

@bebraw will definitely reach back soon! many thanks for reactabular and being a terrific open source maintainer in the React community! 💯 🏅 👏 👏 👏

ohadlevy

first of all - awesome :)
I've left a couple of questions in line. whats the next steps for this pr? (e.g. testsing etc)

ohadlevy · 2017-12-26T08:43:04Z

src/components/DataTable/__mocks__/mockSortableTable.js

+
+    // Point the transform to your rows. React state can work for this purpose
+    // but you can use a state manager as well.
+    const getSortingColumns = () => this.state.sortingColumns || {};


Should be props or state?

I think we can probably provide as props which get converted to state (and tracked in componentWillRecieveProps) in a more abstract implementation. We can probably abstract a lot of this to save the consumer some typing in some use cases. Was thinking the same...

I.e I think some use cases will prefer something quite simple where client sorting is enabled via Boolean “sort” prop, and others more specific where we leave everything open to consumer as we do here. Thoughts?

i think we can probably use Mike T's controlled HOC pattern for this. I will circle back once that's in. This is not super pressing just yet... you will have the option downstream.

ohadlevy · 2017-12-26T08:43:40Z

src/components/DataTable/__mocks__/mockSortableTable.js

+            sortingOrder,
+            selectedColumn,
+          }),
+        });


Nice, what about server side sorting?

Technically Reactabular should allow that as there's nothing frontend specific in the sorting logic. I assume you would apply sort before hydrating the data.

@ohadlevy good point. We can simply provide click callbacks when server side sorting/paging and ignore client paging/sortabular there. I am guessing we will have a lot of variety here so will keep it flexible.

priley86 · 2018-01-04T13:47:35Z

good morning, happy 2018 🍾

I've made a few updates here... and have a few questions.

rebased w/ feat(Form): Add form related components #131 as I needed a lot of Form components for the pagination. Can we try to merge that one soon?
broke this into three components: Table, DataTable, and Pagination. The reactabular-table implementation is light enough for basic tables it seems, and I have broken the smaller components into their stateless components which can be shared across. The Pagination row/client side paginator should be reusable for card view/list view/etc... thoughts?
I decided to mimic reactabular's pagination API, but self implement it. The react-pagify and reactabular pagination components are very different from our design, and are quite simple to write ourself. I left an example of client side sorting and an example of sorting+paging as to not conflate the two...thoughts on combining them or leaving them separate? I will write an example for server side sort/page next (it should be much simpler).
I noticed some styles for the DropdownButton component are different than the ones we have in Core for bootstrap-select. I made some minor tweaks, but I think we'll need some enhancements either here or in Core. @jgiardino do you mind playing around with the pagination "per page" dropdown css when you get a chance?

Some things I am still working on:

empty state
column visibility example
row selections
a11y attributes
server side example
possibly a "dumbed down/easy mode" component that does client side paging/sorting for you and accepts boolean sort args on each column. I don't know if it's worth doing this yet, but I know this use case will probably exist and make life easier for some...

Should we start looking at integrating Filter or a Toolbar next? My vote is to merge this PR first and do those after...

updated storybook is here:
https://rawgit.com/priley86/patternfly-react/reactabular-storybook/index.html

jeff-phillips-18 · 2018-01-05T12:43:29Z

@priley86 I think the toolbar/filter integration should be part of the demo app. We could write advanced storybook stories as well if that is desired.

priley86 · 2018-01-08T17:36:31Z

will rebase Forms + Empty State next and try to finish this one out...

bkearney · 2018-01-10T18:44:48Z

@priley86 @ohadlevy how close is this? I would like this for foreman version 1.18.

priley86 · 2018-01-10T20:07:12Z

@bkearney i'm going to post some updates here later today... here's what i've done locally:

rebased w/ latest
added a11y attributes
row selection example alongside page + sort example is added
refactored a bit to remove some reactabular dependencies and make them optionalDependencies (you only need sortabular if you are doing client side sorting, for example)
added some more useful stateless functional components to go alongside the table row selection cells

Just doing some local testing / cleanup at this point...

I will push this up shortly for review and we can decide if it's worth merging soon, although I prefer a few more days for review here. I think the main idea here is you will have a lot of flexibility on what you want to do (as server side sorting + paging is almost entirely on the downstream and can be simplified a great deal from client side sorting + paging). I plan to add examples of both of those this week to clarify the usage of reactabular... (as it's quite a bit different if doing things on the server). There are plenty of other use cases we can demonstrate, but I'll have to discuss those first.

Hope this gives some context but my aim is to have something ready to merge this week as to open up the door for downstreams to get started...

priley86 · 2018-01-10T21:18:08Z

i've updated w/ my latest local copy here. Still working on some of the things mentioned above, but this should give the idea.

updated storybook:
https://rawgit.com/priley86/patternfly-react/reactabular-storybook/index.html

priley86 · 2018-01-10T21:31:48Z

src/components/DataTable/constants.js

+  FIRST: 'asc',
+  asc: 'desc',
+  desc: 'asc'
+};


leaving this named constants.js for now... will circle back and update all of them in #159

priley86 · 2018-01-10T21:36:30Z

src/components/DataTable/__mocks__/mockPaginationTable.js

+            },
+            transforms: [sortable],
+            formatters: [sortingFormat],
+            customFormatters: [sortableHeaderCell]


customFormatters is only due to the fact that reactabular sortabular formatters do not appear to support wrapped formatters (there is a conflict when using the sort formatter + our own formatter causing sort click / onSort to not fire). I will follow up with @bebraw on this, as ideally, we can just support formatters: [sortingFormat, sortableHeaderCell]. I've mimicked this closely using my own custom formatter in addition to reactabular formatters for now.

priley86 · 2018-01-12T04:31:48Z

few more updates today:
2fd5c38

added some datatable / pagination snapshots
added i18n support to paginator (taken from Fixes #123 - add PaginationRow component #162)
exposed some common formatters in data table constants

priley86 · 2018-01-15T02:20:23Z

i've added the following in my last commit:

Broke out PaginationRow components and added PaginationRowButtonGroup to extend ButtonGroup with .pagination-pf-pagesize class. I've also updated the PF Core PR for this here.
Made all data table formatters consumable/sharable. Formatters which rely on state have been moved to customFormatters so we can pass any arguments we'd like.
Added Server Side pagination example with mockServerApi. Reorganized stories to say "Client side" and "Server side" respectively. Logged server api actions in the Actions addon.
Added Table.Checkbox per Checkbox Support #119
Added more snapshots to ensure we have coverage of all new components

updated storybook:
https://rawgit.com/priley86/patternfly-react/reactabular-storybook/index.html

I'd like to merge this one soon as I have some other upcoming work this week (also this PR is growing quickly). I think we can revisit empty states in another story/another PR which integrates filters and toolbar too possibly?

tstrachota · 2018-01-24T13:37:14Z

@priley86 nope, that was all I found so far.

The "per page" text position is still wrong. Can you check it, please:

jeff-phillips-18 · 2018-01-26T14:42:29Z

Alignment looks fine to me. I don't see the need for the   spacing looks fine without it.

jgiardino · 2018-01-26T19:00:43Z

I'm also not seeing issues with alignment (I checked Chrome, FF, and Safari). And I agree that we don't need the  .

* exposes some common datatable formatters in constants * adds i18n support to paginator

priley86 · 2018-01-26T19:32:35Z

removed fixed space
rebased
redeployed SB

jgiardino · 2018-01-26T20:09:46Z

I also checked just now with IE 10 & 11.

@tstrachota What browser and version were you using?

tstrachota · 2018-01-29T09:33:32Z

It turned out to be issue on my side. My pf-react master apparently wasn't up to date. The alignment is fine when I test the PR on top of the latest master.

I'm sorry for the confusion. 👍 to merge from my side. Good job @priley86 !

priley86 · 2018-01-29T15:46:21Z

thanks very much for the reviews and working through this one with me @tstrachota 👍

jeff-phillips-18 · 2018-01-29T18:33:26Z

Any further comments @jgiardino or anyone else or is this good to merge?

jeff-phillips-18 · 2018-01-29T21:05:17Z

Missed @jgiardino's LGTM above. Going to go ahead and merge this one. Been a long process, thanks @priley86 for all your work on this!

tstrachota · 2018-01-30T10:49:30Z

Good job everybody!

jgiardino added the review label Dec 23, 2017

priley86 force-pushed the reactabular branch from 0cd3eb7 to 53bdf31 Compare December 23, 2017 18:40

This was referenced Dec 23, 2017

WIP feat(tables): adds react-bootstrap-table and patternfly tables #134

Closed

WIP feat(dataTables): Add data table components #75

Closed

priley86 changed the title ~~feat(tables): adds reactabular and patternfly tables examples~~ WIP feat(tables): adds reactabular and patternfly tables examples Dec 23, 2017

ohadlevy reviewed Dec 27, 2017

View reviewed changes

priley86 force-pushed the reactabular branch from 53bdf31 to 7a8830b Compare January 4, 2018 13:34

jeff-phillips-18 added the Do Not Merge label Jan 5, 2018

priley86 force-pushed the reactabular branch from 7a8830b to 4ae1f3f Compare January 10, 2018 21:16

priley86 requested review from cdcabrera and jeff-phillips-18 January 10, 2018 21:26

priley86 commented Jan 10, 2018

View reviewed changes

priley86 mentioned this pull request Jan 11, 2018

Fixes #123 - add PaginationRow component #162

Closed

priley86 mentioned this pull request Jan 12, 2018

fix(pagination): minor css for react-bootstrap pagination dropdown patternfly/patternfly-3#910

Merged

10 tasks

priley86 force-pushed the reactabular branch from 672d810 to 719077a Compare January 12, 2018 21:50

priley86 removed the Do Not Merge label Jan 15, 2018

priley86 changed the title ~~WIP feat(tables): adds reactabular and patternfly tables examples~~ feat(tables): adds reactabular and patternfly tables examples Jan 15, 2018

priley86 force-pushed the reactabular branch from 8f3eca4 to 0cc9582 Compare January 15, 2018 02:22

priley86 added 15 commits January 26, 2018 14:19

feat(tables): adds reactabular and patternfly tables examples

9deb6d1

added row selection and row selection components

0b5e201

* adds pagination and datatable snapshot tests

9fd7a3c

* exposes some common datatable formatters in constants * adds i18n support to paginator

add server side paging example

f476842

adds pagination.less

fa6dcbc

added story source for datatable examples

2922579

update story sources once more

15c0acb

added stateful paginator/pagination row

d9d98b7

scope all reactabular components from Table instead

4cc28c9

adds TableDropdownKebab, story descriptions, i18n fixes

6a14b09

adds noops and default props to simplify jsx

978894c

fix client pagination example, add per page spacing

7055472

update snapshots

643bdde

removed state from paginator

1b237e2

remove per page non breaking space

08ddd9f

priley86 force-pushed the reactabular branch from df3e18d to 08ddd9f Compare January 26, 2018 19:26

jeff-phillips-18 approved these changes Jan 26, 2018

View reviewed changes

jeff-phillips-18 merged commit e664275 into patternfly:master Jan 29, 2018

jgiardino removed the review label Jan 29, 2018

priley86 mentioned this pull request Feb 12, 2018

Enhancement - Pagination previous/next paging #217

Closed

feat(tables): adds reactabular and patternfly tables examples #143

feat(tables): adds reactabular and patternfly tables examples #143

Uh oh!

Conversation

priley86 commented Dec 23, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

serenamarie125 commented Dec 23, 2017

Uh oh!

bebraw commented Dec 26, 2017

Uh oh!

priley86 commented Dec 27, 2017

Uh oh!

ohadlevy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

priley86 commented Jan 4, 2018

Uh oh!

jeff-phillips-18 commented Jan 5, 2018

Uh oh!

priley86 commented Jan 8, 2018

Uh oh!

bkearney commented Jan 10, 2018

Uh oh!

priley86 commented Jan 10, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

priley86 commented Jan 10, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

priley86 Jan 10, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

priley86 commented Jan 12, 2018

Uh oh!

priley86 commented Jan 15, 2018

Uh oh!

tstrachota commented Jan 24, 2018

Uh oh!

jeff-phillips-18 commented Jan 26, 2018

Uh oh!

jgiardino commented Jan 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

priley86 commented Jan 26, 2018

Uh oh!

jgiardino commented Jan 26, 2018

Uh oh!

tstrachota commented Jan 29, 2018

Uh oh!

priley86 commented Jan 29, 2018

Uh oh!

jeff-phillips-18 commented Jan 29, 2018

Uh oh!

jeff-phillips-18 commented Jan 29, 2018

Uh oh!

tstrachota commented Jan 30, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

priley86 commented Dec 23, 2017 •

edited

Loading

priley86 commented Jan 10, 2018 •

edited

Loading

priley86 Jan 10, 2018 •

edited

Loading

jgiardino commented Jan 26, 2018 •

edited

Loading