Skip to content

feat(DataTable): add support for actions and different layouts - #2953

Merged
joshblack merged 15 commits into
mainfrom
feat/add-table-layouts
Mar 1, 2023
Merged

feat(DataTable): add support for actions and different layouts#2953
joshblack merged 15 commits into
mainfrom
feat/add-table-layouts

Conversation

@joshblack

@joshblackjoshblack commented Feb 28, 2023•
edited
Loading

Copy link
Copy Markdown
Member

Closeshttps://github.com/github/primer/issues/1889

Add support for the different layouts for a DataTable (excluding filter). This adds a couple of stories that show support for the table with both a single action and multiple actions. It also includes support for a footer but this will be implemented by pagination

This PR adds two new components to accomplish this: Table.Divider and Table.Actions. It extends Table.Container to be a CSS Grid with different areas for the parts of the table.

Feedback

I would love to hear what you all think about this approach for the layout and if there are any other alternatives that I should explore here or that you suggest using!

If folks feel good with this, I'll make sure to add in some quick tests and docs examples for them 👍

Changelog

New

  • Add Table.Divider for rendering a divider in a table
  • Add Table.Actions for rendering a group of actions for a table
  • Add stories for the following situations:
    • With action
    • With action only (no title)
    • With actions
    • With actions only (no title)

Changed

  • Add support for sx prop to Table.Container
  • Table.Container is now a CSS Grid
  • Add .Table class name for Table component

Removed

@joshblack
joshblack requested review from a team and langermankFebruary 28, 2023 22:13
@changeset-bot

changeset-botBot commented Feb 28, 2023•
edited
Loading

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 2ec85f0

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
NameType
@primer/reactMinor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions

github-actionsBot commented Feb 28, 2023•
edited
Loading

Copy link
Copy Markdown
Contributor

size-limit report 📦

PathSize
dist/browser.esm.js94.49 KB (0%)
dist/browser.umd.js95.06 KB (0%)

@joshblackjoshblack added the skip changeset This change does not need a changelog label Feb 28, 2023
@github-actions
github-actionsBot temporarily deployed to storybook-preview-2953 February 28, 2023 22:20 Inactive
@github-actions
github-actionsBot temporarily deployed to storybook-preview-2953 February 28, 2023 22:21 Inactive
@joshblack

joshblack commented Feb 28, 2023

Copy link
Copy Markdown
MemberAuthor

Quick question for you @mperrotti, should the title of the table align to the top edge of the actions container? Or do we want them to be aligned vertically centered?

Top-aligned

Screenshot 2023-02-28 at 4 21 46 PM

Centered
Screenshot 2023-02-28 at 4 21 55 PM

@joshblack
joshblack temporarily deployed to github-pages February 28, 2023 22:26 — with GitHub Actions Inactive
@github-actions
github-actionsBot temporarily deployed to storybook-preview-2953 February 28, 2023 22:27 Inactive
langermank
langermank approved these changes Feb 28, 2023
View reviewed changes

@langermanklangermank left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Your CSS is real nice ✨ love the use of grid areas! Stories look nice too

colebemis
colebemis approved these changes Mar 1, 2023
View reviewed changes

@colebemiscolebemis left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

💖

@mperrotti

mperrotti commented Mar 1, 2023

Copy link
Copy Markdown
Contributor

should the title of the table align to the top edge of the actions container? Or do we want them to be aligned vertically centered?

Vertically centered is good. I know PageHeader does top-aligned, but that's because they have a different set of layout problems where vertically-centered is more likely to create weird whitespace.

mperrotti
mperrotti approved these changes Mar 1, 2023
View reviewed changes
Comment threadsrc/DataTable/Table.tsx
const StyledTableContainer = styled.div`
display: grid;
grid-template-columns: 1fr 1fr;
grid-template-areas:

@mperrottimperrottiMar 1, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

😍 great use of grid

Base automatically changed from feat/add-table-datatable-to-drafts to mainMarch 1, 2023 21:47
@joshblackjoshblack removed the skip changeset This change does not need a changelog label Mar 1, 2023
@github-actions
github-actionsBottemporarily deployed to storybook-preview-2953 March 1, 2023 22:03 Inactive
@joshblack
joshblack added this pull request to the merge queueMar 1, 2023
Merged via the queue into main with commit 91688a4Mar 1, 2023
@joshblack
joshblack deleted the feat/add-table-layouts branch March 1, 2023 22:17
@primer-cssprimer-css mentioned this pull request Mar 1, 2023
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

3 more reviewers
@mperrottimperrottimperrotti approved these changes
@colebemiscolebemiscolebemis approved these changes
@langermanklangermanklangermank approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@joshblack@mperrotti@colebemis@langermank