FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix(table): set height auto modifier on table row by boaz0 · Pull Request #3133 · patternfly/patternfly-react · GitHub

/ patternfly-react Public

fix(table): set height auto modifier on table row - #3133

Merged
tlabaj merged 1 commit into
patternfly:masterfrom
boaz0:closes_3132
Oct 17, 2019
ConversationCommits (1)ChecksFiles changed
Merged

fix(table): set height auto modifier on table row#3133
tlabaj merged 1 commit into
patternfly:masterfrom
boaz0:closes_3132

Conversation

boaz0 commented Oct 13, 2019 •
edited by jenny-s51
Loading

Copy link
Copy Markdown
Member

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 commented Oct 13, 2019

Copy link
Copy Markdown
Collaborator

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

codecov-io commented Oct 13, 2019 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

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

@@            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.

mcoker previously approved these changes Oct 13, 2019
View reviewed changes

mcoker 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

lgtm!! thanks @boaz0!! 😁👍

jenny-s51 previously approved these changes Oct 15, 2019
View reviewed changes

jenny-s51 left a comment •
edited
Loading

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

jenny-s51 mentioned this pull request Oct 15, 2019
Closed

jenny-s51 commented Oct 15, 2019

Copy link
Copy Markdown
Contributor

will also close #1433

dlabrecq previously approved these changes Oct 16, 2019
View reviewed changes
karelhala reviewed Oct 16, 2019
View reviewed changes
Comment thread packages/patternfly-4/react-table/src/components/Table/utils/decorators/cellHeightAuto.ts Outdated
Comment thread packages/patternfly-4/react-table/src/components/Table/Table.md
EmptyStateTable = () => {
const columns = ['Repositories', 'Branches', 'Pull request', 'Workspaces', 'LastCommit']
const rows = [{
heightAuto: true,

karelhala Oct 16, 2019

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

@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.

fix(table): set height auto modifier on table row
4f44319 
Signed-off-by: Boaz Shuster <boaz.shuster.github@gmail.com>
Co-authored-by: Jenny Shandelman <jshandel@redhat.com>
boaz0 dismissed stale reviews from dlabrecq, jenny-s51, and mcoker via 4f44319 October 16, 2019 13:21
tlabaj approved these changes Oct 17, 2019
View reviewed changes

tlabaj 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM

dlabrecq approved these changes Oct 17, 2019
View reviewed changes

dlabrecq 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM

tlabaj merged commit 6e89ebe into patternfly:master Oct 17, 2019

patternfly-build commented Oct 17, 2019

Copy link
Copy Markdown
Collaborator

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! 🎉

jenny-s51 mentioned this pull request Oct 28, 2019
Closed
boaz0 deleted the closes_3132 branch April 24, 2020 06:40
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

mcoker mcoker left review comments
seanforyou23 Awaiting requested review from seanforyou23
kmcfaul Awaiting requested review from kmcfaul
+4 more reviewers
karelhala karelhala approved these changes
dlabrecq dlabrecq approved these changes
tlabaj tlabaj approved these changes
jenny-s51 jenny-s51 left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

seanforyou23

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[PF4] [react-table] auto modifier is set on td instead of tr

9 participants

Back | FazBrowse Home | New Git URL