-
Notifications
You must be signed in to change notification settings - Fork 350
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
feat(List): Convert list to TypeScript #1946
Conversation
PatternFly-React preview: https://1946-pr-patternfly-react-patternfly.surge.sh |
022d2df
to
dac5f27
Compare
@rebeccaalpert can you also add integration tests for this component as outlined on this README? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also add integration tests for ListVariant, List, ListProps, ListItem, and ListItemProps.
packages/patternfly-4/react-core/src/components/List/ListItem.tsx
Outdated
Show resolved
Hide resolved
packages/patternfly-4/react-core/src/components/List/__snapshots__/List.test.js.snap
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome work! Can we use git mv
to rename the primary component files instead of deleting and recreating them? If we can do this, we'll retain the ability to trace the history of these modules going forward which is important for tracking down when certain changes were made, by who, and why. Looks like you've done this for may of the component files already, if we could give the same treatment for the others that would be great.
dac5f27
to
0cdb560
Compare
Codecov Report
@@ Coverage Diff @@
## master #1946 +/- ##
==========================================
- Coverage 81.84% 81.81% -0.04%
==========================================
Files 628 629 +1
Lines 7112 7116 +4
Branches 236 240 +4
==========================================
+ Hits 5821 5822 +1
Misses 1163 1163
- Partials 128 131 +3
Continue to review full report at Codecov.
|
Added demo and test. I also addressed @dlabaj's PR feedback. @seanforyou23, as noted on Slack, git mv doesn't seem to net a different outcome, so I'm leaving that as-is for now. |
0c9ba87
to
86b2327
Compare
86b2327
to
d5ca189
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great Rebecca.
Converted to TypeScript.
Fixes #1994.