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

fix(data-table): move wrapHeaderLabel to become a table prop #1309

Merged
merged 4 commits into from
Apr 1, 2020

Conversation

jonnybel
Copy link
Contributor

@jonnybel jonnybel commented Apr 1, 2020

Summary

The setting added in #1305 becomes general for the whole table, instead of per-column.

Why?

If one column header breaks into several lines, it will increase the height for all other columns as well, so it makes more sense for the setting to be general: either all cells keep their default height, or they can all use the available vertical space.

Also, it's easier to enable the option using a single prop instead of having to set the property for every individual column.

The setting becomes general for the whole table, instead of per-column.
@jonnybel jonnybel added the 💅 Type: Enhancement Improves existing code label Apr 1, 2020
@jonnybel jonnybel self-assigned this Apr 1, 2020
@vercel
Copy link

vercel bot commented Apr 1, 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/lec3yrxm9
✅ Preview: https://ui-kit-git-ja-table-nowrap-all-columns.commercetools.now.sh

@jonnybel jonnybel force-pushed the ja-table-nowrap-all-columns branch from 3a224c8 to 7b3b9b8 Compare April 1, 2020 15:21
@kodiakhq kodiakhq bot merged commit af9a4a6 into master Apr 1, 2020
@kodiakhq kodiakhq bot deleted the ja-table-nowrap-all-columns branch April 1, 2020 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💅 Type: Enhancement Improves existing code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants