-
Notifications
You must be signed in to change notification settings - Fork 4
Add type definitions #51
base: release/2.0.4
Are you sure you want to change the base?
Conversation
tinhvqbk
commented
Apr 24, 2022
- Add type definitions for this package in src/index.d.ts for the current version 2.0.3
- Update scripts/build.js, .eslintignore
|
Pls update CHANGELOG.md @tinhvqbk |
Updated |
namdaoduy
left a comment
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.
LGTM! Thanks a lot, @tinhvqbk 🚀
src/index.d.ts
Outdated
| | 'information_subtle' | ||
| | 'negative' | ||
| | 'negative_subtle'; | ||
| textClassName: string | string[]; |
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.
This prop should be optional
src/index.d.ts
Outdated
|
|
||
| export interface BubbleChatProps extends BasicProps { | ||
| isTyping?: boolean; | ||
| text?: string; |
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.
text can receive html element too, like the embed html in study_webapp ExpertMessage.tsx
src/index.d.ts
Outdated
| | 'transparentLight'; | ||
| avatar?: string | FuncType; | ||
| time?: string | FuncType; | ||
| options?: OptionType; |
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.
OptionType[]
src/index.d.ts
Outdated
| textClassName?: string | string[]; | ||
| actionBar?: React.ReactNode; | ||
| actionBarClassName?: string | string[]; |
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.
Why textClassName and actionBarClassName can receive array of string in the first place?
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.
Agree, should remove string[] in type
src/index.d.ts
Outdated
| disabled?: boolean; | ||
| nonUppercase?: boolean; | ||
| onlyIcon?: boolean; | ||
| textClassName?: string | string[]; |
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.
Same question above
| export interface FileAttachmentProps extends BasicProps { | ||
| fileType?: | ||
| | 'undefined' | ||
| | 'text' | ||
| | 'image' | ||
| | 'code' | ||
| | 'spreadsheet' | ||
| | 'query' | ||
| | 'powerpoint'; | ||
| fileTypeLabel?: string | FuncType; | ||
| show?: boolean; | ||
| onClose?: () => void; | ||
| closeButton?: boolean; | ||
| actionLeft?: FuncType; | ||
| actionRight?: FuncType; |
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.
Miss filename
| as?: React.ElementType; | ||
| } | ||
| export interface FormFeedbackProps extends BasicProps { | ||
| type: 'valid' | 'invalid'; |
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.
Optional
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.
Should required to more clearly what is Feedback type
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.
@tinhvqbk But I though this prop had default value which is valid already, ref: https://github.dev/tinhvqbk/aha-react/blob/5a702c231c252e3769f2c9b2e32c8922588007fd/src/components/Form/Feedback.js#L19-L23
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.
I think it bug on original source code. The prop type made FormFeedback more clearly.
Ex:
<Form.Feedback>Message</Form.Feedback>
<Form.Feedback type="valid"></Form.Feedback>
What is different? Which code is clean and easy to understand?
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.
Fare point, just to make sure there is no place using this Form.Feedback without passing type prop in all projects
src/index.d.ts
Outdated
| export interface FormLabelProps extends BasicProps { | ||
| sizeLabel?: InputSize; | ||
| required?: boolean; | ||
| htmlFor: string; |
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.
Optional
src/index.d.ts
Outdated
| export interface FormInputProps extends BasicProps { | ||
| type?: string; | ||
| value?: string | number; | ||
| id: string; |
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.
Optional
src/index.d.ts
Outdated
| } | ||
| export interface FormSelectProps extends BasicProps { | ||
| value?: string | number; | ||
| id: string; |
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.
Optional