Inline edit for Table - #3581
Conversation
Sorry, something went wrong.
|
PatternFly-React preview: https://patternfly-react-pr-3581.surge.sh |
Sorry, something went wrong.
Codecov Report
@@ Coverage Diff @@
## master #3581 +/- ##
=========================================
- Coverage 71.29% 70.89% -0.4%
=========================================
Files 782 782
Lines 10527 10599 +72
Branches 2279 2299 +20
=========================================
+ Hits 7505 7514 +9
- Misses 2596 2654 +58
- Partials 426 431 +5
Continue to review full report at Codecov.
|
Sorry, something went wrong.
mcarrano
left a comment
There was a problem hiding this comment.
I did not test extensively, but from what I can see, the behavior looks good @seanforyou23 There was just one visual thing I'm questioning. Why doesn't the edit (pencil) icon align vertically with the kabob in the normal state?
Also, I think the example would make more sense if all of the rows were initialized to non-edit state since that's normally how the table would be presented. A user could then click the edit icon to place any or all rows into edit mode.
Sorry, something went wrong.
mattnolting
left a comment
There was a problem hiding this comment.
LGTM! Great work @seanforyou23!
Sorry, something went wrong.
tlabaj
left a comment
There was a problem hiding this comment.
So looking at this
Sorry, something went wrong.
There was a problem hiding this comment.
Took a look what was done in core... this doesn't seem like it maps easily to a react component. Might want to go back to core to reevaluate the implementation. Seems more like a modifier then a component... or some combination of both.
Sorry, something went wrong.
mcarrano
left a comment
There was a problem hiding this comment.
I am still seeing the Edit icon bottom justified in the row. Should be top-justified like the Kabob icon.
Sorry, something went wrong.
dlabrecq
left a comment
There was a problem hiding this comment.
The edit icon position still appears to be off a bit
Sorry, something went wrong.
mcarrano
left a comment
There was a problem hiding this comment.
Looks great @seanforyou23 !
Sorry, something went wrong.
mcarrano
left a comment
There was a problem hiding this comment.
Still looks good!
Sorry, something went wrong.
mcarrano
left a comment
There was a problem hiding this comment.
One more time...
Sorry, something went wrong.
tlabaj
left a comment
There was a problem hiding this comment.
LGTM
Sorry, something went wrong.
|
Your changes have been released in:
Thanks for your contribution! 🎉 |
Sorry, something went wrong.
|
⚠️ Please install the Codecov Report❌ Patch coverage is 39.81481% with 65 lines in your changes missing coverage. Please review.
@@ Coverage Diff @@
## master #3581 +/- ##
==========================================
- Coverage 71.29% 70.96% -0.33%
==========================================
Files 782 785 +3
Lines 10527 10632 +105
Branches 2279 2313 +34
==========================================
+ Hits 7505 7545 +40
- Misses 2596 2656 +60
- Partials 426 431 +5
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Sorry, something went wrong.
What: This PR adds row-level inline edit feature and supporting tests to react-table and react-integration packages. It also fixes an issue where rows used non-unique react keys by default and a type issue in RowWrapper, along with some other type enhancements that (hopefully) don't introduce any breaking changes. We've also added a row-level validation hook which can be used to apply arbitrary rulesets for any given row as defined by the user. This validation hook only runs on submission of the row edit, not onChange of the individual inputs, there are plans to add validation onBlur in a future effort.
Should help close #3367 #2679 #3357 and #3058