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(table): set height auto modifier on table row #3133

Merged
merged 1 commit into from Oct 17, 2019

Conversation

@boaz0
Copy link
Member

boaz0 commented Oct 13, 2019

What:

close #3132

I mistakenly added the height auto modifier to td element instead to tr.

This PR fixes this by adding the HeightAuto attribute to row item.
In addition to that, added a deprecation warning message to whom is using cellHeightAuto.

More tests and an example were added.

//cc @jenny-s51 @mcoker @karelhala @tlabaj @redallen

@patternfly-build

This comment has been minimized.

Copy link
Contributor

patternfly-build commented Oct 13, 2019

PatternFly-React preview: https://patternfly-react-pr-3133.surge.sh

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Oct 13, 2019

Codecov Report

Merging #3133 into master will increase coverage by 0.09%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3133      +/-   ##
==========================================
+ Coverage   68.94%   69.03%   +0.09%     
==========================================
  Files         858      858              
  Lines       23590    23539      -51     
  Branches     1887     1879       -8     
==========================================
- Hits        16263    16250      -13     
+ Misses       6367     6336      -31     
+ Partials      960      953       -7
Flag Coverage Δ
#misc 95.45% <ø> (ø) ⬆️
#patternfly3 69.23% <ø> (ø) ⬆️
#patternfly4 68.12% <100%> (+0.18%) ⬆️
Impacted Files Coverage Δ
...rnfly-4/react-table/src/components/Table/Table.tsx 91.25% <ø> (-0.11%) ⬇️
...-4/react-table/src/components/Table/RowWrapper.tsx 78% <100%> (+0.44%) ⬆️
...omponents/Table/utils/decorators/cellHeightAuto.ts 100% <100%> (ø) ⬆️
...ernfly-4/react-table/src/components/Table/Body.tsx 72.91% <100%> (+0.57%) ⬆️
.../react-table/src/components/Table/SelectColumn.tsx 71.42% <0%> (-3.58%) ⬇️
...-4/react-table/src/components/Table/SortColumn.tsx 76.47% <0%> (-1.31%) ⬇️
...ct-charts/src/components/ChartUtils/chart-theme.ts 65.62% <0%> (-0.34%) ⬇️
...nfly-4/react-core/src/components/Wizard/Wizard.tsx 53.84% <0%> (-0.29%) ⬇️
...y-3/patternfly-react/src/components/Table/Table.js 95.71% <0%> (-0.04%) ⬇️
... and 6 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 031b8f7...4f44319. Read the comment docs.

@boaz0 boaz0 force-pushed the boaz0:closes_3132 branch from e6fdae7 to b136c38 Oct 13, 2019
Copy link
Contributor

mcoker left a comment

lgtm!! thanks @boaz0!! 😁👍

Copy link
Contributor

jenny-s51 left a comment

Thank you Boaz! Nice work on this 👍 Closing my PR for an empty state table at #3126

@jenny-s51

This comment has been minimized.

Copy link
Contributor

jenny-s51 commented Oct 15, 2019

will also close #1433

EmptyStateTable = () => {
const columns = ['Repositories', 'Branches', 'Pull request', 'Workspaces', 'LastCommit']
const rows = [{
heightAuto: true,

This comment has been minimized.

Copy link
@karelhala

karelhala Oct 16, 2019

Contributor

@priley86 what do you think about introducing row decorators? Similiar to how cell formatters work. This can be potentially problem in future when we'll update API and we will have to keep it intact.

Signed-off-by: Boaz Shuster <boaz.shuster.github@gmail.com>
Co-authored-by: Jenny Shandelman <jshandel@redhat.com>
@boaz0 boaz0 dismissed stale reviews from dlabrecq, jenny-s51, and mcoker via 4f44319 Oct 16, 2019
@boaz0 boaz0 force-pushed the boaz0:closes_3132 branch from b136c38 to 4f44319 Oct 16, 2019
@tlabaj
tlabaj approved these changes Oct 17, 2019
Copy link
Contributor

tlabaj left a comment

LGTM

Copy link
Member

dlabrecq left a comment

LGTM

@tlabaj tlabaj merged commit 6e89ebe into patternfly:master Oct 17, 2019
8 checks passed
8 checks passed
ci/circleci: build Your tests passed on CircleCI!
Details
ci/circleci: build_integration Your tests passed on CircleCI!
Details
ci/circleci: build_pf3_docs Your tests passed on CircleCI!
Details
ci/circleci: build_pf4_docs Your tests passed on CircleCI!
Details
ci/circleci: lint Your tests passed on CircleCI!
Details
ci/circleci: test_jest_other Your tests passed on CircleCI!
Details
ci/circleci: test_jest_pf4 Your tests passed on CircleCI!
Details
ci/circleci: upload_docs Your tests passed on CircleCI!
Details
@patternfly-build

This comment has been minimized.

Copy link
Contributor

patternfly-build commented Oct 17, 2019

Your changes have been released in:

  • @patternfly/react-inline-edit-extension@2.11.98
  • demo-app-ts@3.7.12
  • @patternfly/react-integration@3.7.1
  • @patternfly/react-table@2.23.14

Thanks for your contribution! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.