Completed Timeine onScroll Animation #42
Conversation
There was a problem hiding this comment.
1- Remove the vs code file while making PR
2- Rename as Timeline_onScroll_Animation_01
3- In the child-container of before animation, you have done opacity 0 so all I could see was blank screen
4- Since your elements enlarges on scroll add a header text or something because that effect is not visible
Reference:
Opacity 0:
Opacity 1 but no scroll area:
…into Timeline_onScrollAnimation
1- Remove the vs code file while making PR 2- Rename as Timeline_onScroll_Animation_01
|
I have done some corrections . Can you check once again please @nandini7637 |
bhaveyx
left a comment
bhaveyx
left a comment
There was a problem hiding this comment.
Nice work ! But it seems like the component becomes irresponsive below 900px. Kindly try fixing it.
|
@bhavesh-chaudhari bro i checked it but it seems to be working properly . May be its an issue with screen height I guess. Can you please share me the screen height and width both at which things are getting messed ? and also the browser at which you checked. |
|
@Abirpal202049 Timeline Tech Stack Used
Preview2021-12-25.01-59-04.mp4 |
I think the issue is arising irrespective of the height. You can try checking responsiveness on some commonly used resolutions for devices like iphone, kindle fire, ipad, etc. I have checked on firefox and chrome. For ex:-
Please do let me know if i am mistaken somewhere. My suggestion is to try making it look consistent on most standard resolutions for mobile and tabs, so that this component becomes a complete solution on such timeline design. Happy Coding ! |
| @@ -0,0 +1,19 @@ | |||
| window.addEventListener('scroll', () => { | |||
There was a problem hiding this comment.
Try adding comments wherever possible so that it makes your code flow clear to the reader.
There was a problem hiding this comment.
Ok I will try to add comments
| let contentPosition = content.getBoundingClientRect().top; | ||
| let screenPosition = window.innerHeight; | ||
| contentPosition < screenPosition | ||
| ? content.classList.add('active') |
There was a problem hiding this comment.
Avoid using ternary operator if possible since it makes the code difficult to read and understand
There was a problem hiding this comment.
So should i change it to normal if/else syntax or should add a comment there ?
@Ovenoboyo
| @@ -0,0 +1,19 @@ | |||
| window.addEventListener('scroll', () => { | |||
| let contents = document.querySelectorAll('.child-container'); | |||
There was a problem hiding this comment.
Please use "const" instead of "let" wherever possible
| --green-light: #aff1b6; | ||
| } | ||
|
|
||
| header { |
There was a problem hiding this comment.
Avoid styling html tags directly, Instead move these styles to a css class
There was a problem hiding this comment.
Those where just added for user demonstration of the slide animation . No worries i will add classes to them .
sohan9819
requested review from
Abirpal202049,
Ovenoboyo,
bhaveyx and
saikatmu31
|
@bhavesh-chaudhari bro I am still not getting the issue i tried testing it in chrome mozilla and brave . I you are free any time we can get to discord server o discuss about this . 2021-12-27.20-58-29.mp4 |
|
@sohan9819 Hey this seems great 👍 ! Actually there was some issue with my browser settings. Sorry for the inconvenience ! I have added alt to the images which was missing everything else seems fine. Approving this pr, thanks for the contribution. |
|
@bhavesh-chaudhari no problem bro . Thanks for the help 👍 |
|
What is your P.Id @sohan9819 |
|
I didn't understand 😅 ? @Abirpal202049 |
|
Participent Id |
|
@Abirpal202049 sorry bro I dont know about participent id 😅. Can you tell me from where I can get it . |
6 participants
Added the Timeline in Timelines folder .
Preview of the code .
2021-12-25.01-59-04.mp4