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

feat(textinput): support passing a ref by boaz0 · Pull Request #3168 · patternfly/patternfly-react · GitHub

/ patternfly-react Public

feat(textinput): support passing a ref - #3168

Merged
kmcfaul merged 1 commit into
patternfly:masterfrom
boaz0:closes_3153
Dec 2, 2019
ConversationCommits (1)ChecksFiles changed
Merged

feat(textinput): support passing a ref#3168
kmcfaul merged 1 commit into
patternfly:masterfrom
boaz0:closes_3153

Conversation

boaz0 commented Oct 20, 2019

Copy link
Copy Markdown
Member

What:

closes #3153

  • I had to update the tests because of enzyme snapshot (TextInput -> ForwardRef)
  • I added an example for react-hook-form. I am not sure we should have it.
  • I added withInnerRef to the helpers directory. This will make it easier to add support to ref in other component.

//cc @kmcfaul @ia3andy

tlabaj requested a review from kmcfaul October 22, 2019 20:22

codecov-io commented Oct 25, 2019 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Merging #3168 into master will decrease coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #3168      +/-   ##
==========================================
- Coverage   67.52%   67.52%   -0.01%     
==========================================
  Files         896      895       -1     
  Lines       25112    25057      -55     
  Branches     2173     2158      -15     
==========================================
- Hits        16958    16920      -38     
+ Misses       7141     7130      -11     
+ Partials     1013     1007       -6
Flag Coverage Δ
#misc 95.45% <ø> (ø) ⬆️
#patternfly3 69.28% <ø> (ø) ⬆️
#patternfly4 65% <100%> (-0.02%) ⬇️
Impacted Files Coverage Δ
...kages/patternfly-4/react-core/src/helpers/index.ts 100% <100%> (ø) ⬆️
.../react-core/src/components/TextInput/TextInput.tsx 95.55% <100%> (+0.43%) ⬆️
...tternfly-4/react-core/src/helpers/withInnerRef.tsx 100% <100%> (ø)
...mental/components/DataToolbar/DataToolbarGroup.tsx 93.75% <0%> (-6.25%) ⬇️
...nents/DataToolbar/DataToolbarExpandableContent.tsx 95.23% <0%> (-1.06%) ⬇️
...nfly-4/react-core/src/components/Select/Select.tsx 63.87% <0%> (-0.42%) ⬇️
...nfly-4/react-core/src/components/Dropdown/index.ts 100% <0%> (ø) ⬆️
...mental/components/DataToolbar/DataToolbarUtils.tsx 100% <0%> (ø) ⬆️
...ntal/components/DataToolbar/DataToolbarContent.tsx 100% <0%> (ø) ⬆️
...onents/DataToolbar/DataToolbarChipGroupContent.tsx 81.81% <0%> (ø) ⬆️
... and 11 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 78bd323...bad2b9a. Read the comment docs.

patternfly-build commented Oct 25, 2019

Copy link
Copy Markdown
Collaborator

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

nicolethoen reviewed Oct 25, 2019
View reviewed changes

nicolethoen 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

The PR description mentions updating the examples? Was that decided against? or was it overlooked?

I'm also noticing that all the props are missing from the documentation - but I dont think that's related to this PR, thats probably an issue with building the react docs.

Comment thread packages/patternfly-4/react-core/src/helpers/withInnerRef.tsx
@@ -0,0 +1,7 @@
import * as React from 'react';

export function withInnerRef<R, P extends { innerRef?: React.Ref<R> }>(

nicolethoen Oct 25, 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

I can imagine this will be very helpful, thanks!

dlabrecq suggested changes Nov 5, 2019
View reviewed changes

dlabrecq left a comment •
edited
Loading

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

We should have an example, similar to the tabs "separate content" ref example. I don't necessarily see a problem, but refs cannot be passed down as props, so it would be good to also have an example for users.

kmcfaul previously approved these changes Nov 18, 2019
View reviewed changes
dlabrecq suggested changes Nov 19, 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

The code looks good, but have a question about using a primary button to show functionality

Comment thread packages/patternfly-4/react-core/src/components/TextInput/examples/TextInput.md Outdated
feat(textinput): support passing a ref
bad2b9a 
Signed-off-by: Boaz Shuster <boaz.shuster.github@gmail.com>
kmcfaul merged commit 1e1ca74 into patternfly:master Dec 2, 2019

patternfly-build commented Dec 2, 2019

Copy link
Copy Markdown
Collaborator

Your changes have been released in:

  • @patternfly/react-catalog-view-extension@1.1.41
  • @patternfly/react-core@3.125.0
  • @patternfly/react-docs@4.16.47
  • @patternfly/react-inline-edit-extension@2.14.0
  • demo-app-ts@3.13.4
  • @patternfly/react-table@2.24.44
  • @patternfly/react-topology@2.11.30
  • @patternfly/react-virtualized-extension@1.3.43

Thanks for your contribution! 🎉

boaz0 commented Dec 2, 2019

Copy link
Copy Markdown
Member Author

Thanks a lot 😄 🎊

boaz0 deleted the closes_3153 branch December 2, 2019 20:12
jenny-s51 mentioned this pull request Sep 28, 2020
Closed
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

nicolethoen nicolethoen left review comments
kmcfaul kmcfaul approved these changes
+1 more reviewer
dlabrecq dlabrecq approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

kmcfaul

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

TextInput: expose ref of input element with forwardRef

6 participants

Back | FazBrowse Home | New Git URL