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

[UKIT-96] feat(table): add new component DataTable #1233

Merged
merged 63 commits into from
Mar 11, 2020
Merged

Conversation

jonnybel
Copy link
Contributor

@jonnybel jonnybel commented Jan 6, 2020

Summary

The new DataTable component.
UKIT-96

Description

The new table uses CSS-grid, doesn't use virtualization, and tries to keep a similar API as the existing table component.

Check out the latest version and be sure to play around with all the available knobs and column options:
https://ui-kit-git-ja-new-table.commercetools.now.sh/?path=/story/components-table-new--datatable

Feature Checklist

  • Basic Table functionality with the current API, with items and columns definition arrays, and renderItem prop replacing itemRenderer.
  • Table uses all available space
  • Adjustable max and height and width
  • Cell alignment: configurable at table level and column level -- setting at column level takes precedence.
  • Sticky Header
  • "Condensed" mode: compact mode with less spacings for the cells, maximizing the usage of available space
  • Column width is fully adjustable: accepts auto or fixed width as well as a minmax pair (using the css-grid template column syntax).
  • Uses semantic <table> markup under the hood.
  • Truncated text cell option
  • Row hovering
  • Row click
  • Cell click (and option to ignore rowClick)
  • Row Selection (with useRowSelection hook. This can also be done by the consumer, like it is on the old table)
  • Row Sorting (also with a hook)

Follow-up:

  • Add VRTs
  • Test in MC to discover potential bugs and enhancements "in the field"

A heads up about this commit: 0099678

I've changed the API a little bit:

  • items prop renamed to rows, to create less confusion regarding the difference with actual items (cells) of a table. Also, I think it pairs better with the columns prop.
  • Changed the renderItem prop of the table back to itemRenderer, and this becomes the default renderer for each item. The default is set to simply render whatever is the value of an item property (usually it's a string). See: https://github.com/commercetools/ui-kit/pull/1233/files#diff-aa4a4319c62f56d7a3a775b74edbcb06R94
  • Added a renderItem prop to the column definitions. This is the renderer to be used for custom cases per-column, taking precedence over the default itemRenderer. I find it's easier to have this on the column definition (among with others like onClick), instead of having to implement a switch (case) logic on the itemRenderer prop. However, that is still possible which may actually help migrate things from the old table faster.

I dediced for these changes because it makes rendering a table much easier:

  • The only two required props to render the simplest responsive table are columns and rows 🎉

@jonnybel jonnybel added the 🚧 Status: WIP Work in progress label Jan 6, 2020
@jonnybel jonnybel self-assigned this Jan 6, 2020
@vercel
Copy link

vercel bot commented Jan 6, 2020

This pull request is being automatically deployed with ZEIT Now (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://zeit.co/commercetools/ui-kit/izl5997c1
✅ Preview: https://ui-kit-git-ja-new-table.commercetools.now.sh

Copy link
Contributor

@tdeekens tdeekens left a comment

Choose a reason for hiding this comment

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

Nice to see this taking off. The decision of using/building upon the same API is nice to ease the adoption. Can you elaborate maybe - if you have - what an improved API in a dreamy-🦄-world would look like?

Copy link
Member

@emmenko emmenko left a comment

Choose a reason for hiding this comment

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

The package is missing all the package related stuff (package.json, LICENSE, src folder, etc).

Copy link
Member

@emmenko emmenko left a comment

Choose a reason for hiding this comment

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

Code looks good so far! 👌

@jonnybel jonnybel changed the title feat(table): add new table [UKIT-96] feat(table): add new table Jan 10, 2020
@vercel vercel bot temporarily deployed to Preview March 10, 2020 18:13 Inactive
@jonnybel jonnybel changed the title [UKIT-96] feat(table): add new table [UKIT-96] feat(table): add new component DataTable Mar 11, 2020
@jonnybel
Copy link
Contributor Author

Renamed this component to DataTable.

This PR will finally be merged after this. See this comment.

@jonnybel jonnybel added the 🚀 Type: New Feature Something new label Mar 11, 2020
@jonnybel jonnybel added 🚀 Status: ship it and removed 👨‍🎨 Status: UI/UX Review Requires review from design team 🙏 Status: Dev Review Waiting for technical reviews labels Mar 11, 2020
@jonnybel jonnybel merged commit b029c77 into master Mar 11, 2020
@jonnybel jonnybel deleted the ja-new-table branch March 11, 2020 18:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants