Skip to content

Add horizontal alignment to column configuration - #3068

Merged
mperrotti merged 5 commits into
mainfrom
mp/table-col-align
Mar 28, 2023
Merged

Add horizontal alignment to column configuration#3068
mperrotti merged 5 commits into
mainfrom
mp/table-col-align

Conversation

@mperrotti

@mperrottimperrotti commented Mar 23, 2023

Copy link
Copy Markdown
Contributor

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

Screenshots

Kapture 2023-03-23 at 10 25 54

Merge checklist

  • Added/updated tests
  • Added/updated documentation
  • Tested in Chrome
  • Tested in Firefox
  • Tested in Safari
  • Tested in Edge

Take a look at the What we look for in reviews section of the contributing guidelines for more information on how we review PRs.

@mperrotti
mperrotti requested review from a team and joshblackMarch 23, 2023 15:12
@changeset-bot

changeset-botBot commented Mar 23, 2023•
edited
Loading

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: fb61132

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 Mar 23, 2023•
edited
Loading

Copy link
Copy Markdown
Contributor

size-limit report 📦

PathSize
dist/browser.esm.js95 KB (0%)
dist/browser.umd.js95.55 KB (0%)

@mperrotti
mperrotti temporarily deployed to github-pages March 23, 2023 15:19 — with GitHub Actions Inactive
@github-actions
github-actionsBot temporarily deployed to storybook-preview-3068 March 23, 2023 15:20 Inactive
joshblack
joshblack approved these changes Mar 23, 2023
View reviewed changes

@joshblackjoshblack left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks great! Just left a comment for the CSS Selector.

Also wanted to ask if we wanted a shared Alignment or BlockAlignment type after seeing the ToggleSwitch update. I think seeing CellAlignment there was unexpected since I didn't think of it as a cell but totally get wanting alignment values to be consistent across the library/design system 👍

Comment threadsrc/DataTable/Table.tsx Outdated
justify-content: flex-end;
}

.TableHeader[data-cell-align='end'] > button {

@joshblackjoshblackMar 23, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could we add a class to the table sort button instead of using an element selector?

@mperrottimperrottiMar 23, 2023

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Sure!

@mperrotti
mperrotti temporarily deployed to github-pages March 23, 2023 22:19 — with GitHub Actions Inactive
@github-actions
github-actionsBot temporarily deployed to storybook-preview-3068 March 23, 2023 22:19 Inactive
joshblack
joshblack approved these changes Mar 24, 2023
View reviewed changes

@joshblackjoshblack left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🥳

@mperrotti
mperrotti enabled auto-merge March 28, 2023 16:04
@mperrotti
mperrotti added this pull request to the merge queueMar 28, 2023
@github-actions
github-actionsBottemporarily deployed to storybook-preview-3068 March 28, 2023 16:20 Inactive
Merged via the queue into main with commit 09e1133Mar 28, 2023
@mperrotti
mperrotti deleted the mp/table-col-align branch March 28, 2023 16:31
@primer-cssprimer-css mentioned this pull request Mar 28, 2023
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@joshblackjoshblackjoshblack approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@mperrotti@joshblack