Skip to content
This repository was archived by the owner on Apr 26, 2024. It is now read-only.

Enhance the TextField component#156

Merged
DiogoSoaress merged 7 commits intodevelopmentfrom
enhance-text-field
Oct 8, 2021
Merged

Enhance the TextField component#156
DiogoSoaress merged 7 commits intodevelopmentfrom
enhance-text-field

Conversation

@DiogoSoaress
Copy link
Contributor

No description provided.

return {
value: props.value,
onChange: props.onChange,
};
Copy link
Member

@katspaugh katspaugh Oct 4, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a quick feedback, it seems like you suggest to support two sets of incompatible props on the same component.
Would it maybe make sense to expose two different components then?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a pretty common pattern in TypeScript 🤷‍♂️ It's like you do string | number, but a little bit more complex

Use of conditional props to receive an input and use its own onChange instead of a onChange handler passed in the props.

Co-authored-by: Aaron Cook <iamacook@users.noreply.github.com>
@github-actions
Copy link

github-actions bot commented Oct 4, 2021

ESLint Summary View Full Report

Annotations are provided inline on the Files Changed tab. You can also see all annotations that were generated on the annotations page.

Type Occurrences Fixable
Errors 0 0
Warnings 0 0
Ignored 0 N/A
  • Result: ✅ success
  • Annotations: 0 total

Report generated by eslint-plus-action

@iamacook iamacook linked an issue Oct 7, 2021 that may be closed by this pull request
Co-authored-by: DiogoSoaress <DiogoSoaress@users.noreply.github.com>
@iamacook
Copy link
Contributor

iamacook commented Oct 7, 2021

After discussing the props with @mikheevm, @DiogoSoaress and I came up with the above, standardised Props.

@DiogoSoaress DiogoSoaress marked this pull request as ready for review October 7, 2021 14:01
@DiogoSoaress DiogoSoaress requested a review from mmv08 October 7, 2021 14:01
value={value}
color="primary"
onChange={onChange}
readOnly
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is it always true?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch

@DiogoSoaress DiogoSoaress requested a review from mmv08 October 7, 2021 14:50
@DiogoSoaress DiogoSoaress merged commit b5f5ea0 into development Oct 8, 2021
@katspaugh katspaugh deleted the enhance-text-field branch October 8, 2021 08:31
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Enhance [TextField] typing

4 participants