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

feat(TextInput): Convert text input to TypeScript #1914

Merged
merged 7 commits into from Jun 5, 2019

Conversation

@rebeccaalpert
Copy link
Member

rebeccaalpert commented May 2, 2019

Fixes #1908.

@patternfly-build

This comment has been minimized.

Copy link
Contributor

patternfly-build commented May 2, 2019

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented May 2, 2019

Codecov Report

Merging #1914 into master will increase coverage by 0.04%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1914      +/-   ##
==========================================
+ Coverage   81.09%   81.14%   +0.04%     
==========================================
  Files         643      644       +1     
  Lines        7706     7721      +15     
  Branches      451      453       +2     
==========================================
+ Hits         6249     6265      +16     
+ Misses       1257     1256       -1     
  Partials      200      200
Flag Coverage Δ
#patternfly3 84.88% <ø> (ø) ⬆️
#patternfly4 77.01% <100%> (+0.11%) ⬆️
#patternflymisc 95.68% <ø> (ø) ⬆️
Impacted Files Coverage Δ
...fly-4/react-core/src/components/TextInput/index.ts 100% <100%> (ø)
.../react-core/src/components/TextInput/TextInput.tsx 100% <100%> (ø)

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 6c04803...d70a2a2. Read the comment docs.

@nicolethoen

This comment has been minimized.

Copy link
Contributor

nicolethoen commented May 8, 2019

@rebeccaalpert can you also add integration tests for this component as outlined on this README?

@redallen redallen added the TypeScript label May 9, 2019
@rebeccaalpert rebeccaalpert force-pushed the rebeccaalpert:textinput-typescript branch 3 times, most recently from 5ccf1cd to 944fc63 May 14, 2019
@rebeccaalpert

This comment has been minimized.

Copy link
Member Author

rebeccaalpert commented May 14, 2019

Added demo and tests.

@rebeccaalpert rebeccaalpert force-pushed the rebeccaalpert:textinput-typescript branch from 560a6c1 to d5cfc3c May 15, 2019
@patternfly-build

This comment has been minimized.

Copy link
Contributor

patternfly-build commented May 15, 2019

@tlabaj tlabaj added chore 🏠 and removed enhancement 🚀 labels May 15, 2019
@rebeccaalpert rebeccaalpert force-pushed the rebeccaalpert:textinput-typescript branch from d5cfc3c to 2aa2452 May 16, 2019
@rebeccaalpert

This comment has been minimized.

Copy link
Member Author

rebeccaalpert commented May 16, 2019

Updated enum typing as discussed in meeting.

@rebeccaalpert rebeccaalpert requested review from tlabaj and dlabaj May 16, 2019
Copy link
Contributor

dlabaj left a comment

A few comments. Let me know if you have any questions.

@rebeccaalpert rebeccaalpert force-pushed the rebeccaalpert:textinput-typescript branch 9 times, most recently from 126969e to 337b306 May 16, 2019
@dlabaj dlabaj dismissed redallen’s stale review May 17, 2019

Re requesting review

@dlabaj dlabaj requested a review from redallen May 17, 2019
Copy link
Contributor

tlabaj left a comment

Jest Test are failing.

@rebeccaalpert rebeccaalpert force-pushed the rebeccaalpert:textinput-typescript branch 2 times, most recently from 604f7a9 to b3ae4be May 28, 2019
Copy link
Contributor

dlabaj left a comment

LGTM

Copy link
Contributor

tlabaj left a comment

LGTM

@rebeccaalpert rebeccaalpert force-pushed the rebeccaalpert:textinput-typescript branch from 4acbe49 to 9a7740c May 29, 2019
@rebeccaalpert rebeccaalpert force-pushed the rebeccaalpert:textinput-typescript branch from 0045dde to d70a2a2 May 29, 2019
Copy link
Contributor

redallen left a comment

Really close, thanks for your work!

@@ -1,6 +1,6 @@
import React from 'react';

This comment has been minimized.

Copy link
@redallen

redallen May 30, 2019

Contributor

Change the extension on this file to .tsx

/** A callback for when the input value changes. */
onChange?: (value: string, event: React.FormEvent<HTMLInputElement>) => void;
/** Type that the input accepts. */
type?: 'text' | 'date' | 'datetime-local' | 'email' | 'month' | 'number' | 'password' | 'search' | 'tel' | 'time' | 'url';

This comment has been minimized.

Copy link
@redallen

redallen May 30, 2019

Contributor

To allow use of the enum, I believe you need to add | TextInputTypes

const { className, type, value, onChange, isValid, isReadOnly, isRequired, isDisabled, ...props } = this.props;
return (
<input
{...props}

This comment has been minimized.

Copy link
@redallen

redallen May 30, 2019

Contributor

Let's spread the props at the end to be consistent across components.

@tlabaj
tlabaj approved these changes Jun 5, 2019
Copy link
Contributor

tlabaj left a comment

LGTM

@tlabaj tlabaj merged commit 1707236 into patternfly:master Jun 5, 2019
2 checks passed
2 checks passed
ci/circleci: build Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@rebeccaalpert rebeccaalpert deleted the rebeccaalpert:textinput-typescript branch Jun 6, 2019
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.