fixed #445 by adding a animated search component #459
Conversation
|
@e-for-eshaan I have closed the previous PR and made a new one. Please review this one. |
|
|
||
| const AnimatedRepos = () => { | ||
| const [animationIndex, setAnimationIndex] = useState(0); | ||
| const [repos] = useState(popRepos); |
There was a problem hiding this comment.
you can simply use popRepos. It's a global constant and can be utilized without having to pass into a state. Just remove this line of code
There was a problem hiding this comment.
I have made this change, now using just popRepos.
| .placeholder{ | ||
| border: none; | ||
| outline: none; | ||
| right: 0px; |
There was a problem hiding this comment.
element isn't relative, nor absolute, why are we using right: 0px;
There was a problem hiding this comment.
Removed unnecessary styling in the latest commit
| <input | ||
| className={styles.placeholder} | ||
| style={{ backgroundColor: themeColors.primaryAlt }} | ||
| /> |
There was a problem hiding this comment.
Why are you rendering this input? it doesn't seem to show up anywhere.
I guess you need to add some width and height to the <div> component in AnimatedInputWrapper component, and remove input completely.
Feel free to ask more about this.
There was a problem hiding this comment.
used a div instead of input in the latest commit
| @@ -0,0 +1,29 @@ | |||
| .animationWrapper{ | |||
| position: absolute; | |||
There was a problem hiding this comment.
you are using absolute position, but the parent (element wrapping this component) isn't having relative position.
this can cause unexpected behaviour when you are trying to debug.
There was a problem hiding this comment.
Gave position:relative to .placeholder which was the parent element of .animationWrapper
| className={styles.text} | ||
| style={{ bottom: animationIndex * 1.4 + 'em' }} | ||
| > | ||
| {repos.map((item) => ( |
There was a problem hiding this comment.
then this becomes popRepos.map
| ) : !searchFocus.value ? ( | ||
| <AnimatedInputWrapper /> | ||
| ) : ( | ||
| 'org/repo' |
There was a problem hiding this comment.
it should have said Search for org/repo
#448 (comment)
Can you please merge the PR? |
Linked Issue(s)
Closes #445
Acceptance Criteria fulfillment
Proposed changes (including videos or screenshots)
Slide.Animation.mp4