Updated the fade-in effect on reveal of enhanced text and fix other issues on feedback #83
Conversation
| let overlay = null | ||
|
|
||
| React.Children.forEach(children, child => { | ||
| if (child.type && child.type.displayName === 'PromptInputTextarea') { |
There was a problem hiding this comment.
Can we check if child.type is defined once at the top?
There was a problem hiding this comment.
Refactored the code and created separate helper for type checking logic
| {textarea} | ||
| {overlay} | ||
| {actions} | ||
| {React.Children.map(children, child => { |
There was a problem hiding this comment.
Some thoughts here:
- We do something similar above, so we can move type checking logic to a separate helper, wyt?
- Are we doing this to make sure the components that are not textarea, overlay, and actions will go after these three?
There was a problem hiding this comment.
- Refactored the code and created separate helper for type checking logic
- Yes, the first 3 components should be rendered first before the other children because it affects the positioning of the text when it will be displayed by the overlay.
| } | ||
| }, [text]) | ||
|
|
||
| useEffect(() => { |
There was a problem hiding this comment.
Could you add some short comments to all useEffect hooks in this component?
There was a problem hiding this comment.
Maybe it would be good to move all those useEffects to a separate hook?
There was a problem hiding this comment.
Added some comments to the useEffect but did not move into a separate hook since it's only being used by this component.
| overlay.style.position = 'absolute' | ||
| overlay.style.top = paddingTop | ||
| overlay.style.left = paddingLeft | ||
| overlay.style.width = 'calc(100% - ' + (parseInt(paddingLeft) * 2) + 'px)' |
There was a problem hiding this comment.
Could you use template string here?
There was a problem hiding this comment.
Refactored code to use template string.
| {paragraphs.map((paragraph, index) => ( | ||
| <p | ||
| key={index} | ||
| className={`ds-prompt-input__text-reveal-paragraph ${ |
There was a problem hiding this comment.
Refactored code to use classnames
…o check displayName
Docs