Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#16 problems section #18

Merged
merged 5 commits into from
Jan 5, 2023

Conversation

virajchandra51
Copy link
Contributor

Structured previous issue into right folders along with few styling changes
Added problemsData
Added LoadMore and ShowLess button to show all or only 2 problems at a time
Added Modal opening on each problem passing its data to the modal child component
Styling of the modal can be better done once full statements are received.
Added animation wherever possible.
Not able to figure out how to add load more and show less animation, as I'm slicing the array Data rather than controlling single cards

@netlify
Copy link

netlify bot commented Jan 3, 2023

Deploy Preview for codeutsava ready!

Name Link
🔨 Latest commit 82cbd65
🔍 Latest deploy log https://app.netlify.com/sites/codeutsava/deploys/63b7231114bea40008543ec6
😎 Deploy Preview https://deploy-preview-18--codeutsava.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@prakhar-asaiya
Copy link
Contributor

For proper animation you can try to explore some library like react-spring. They usually help with these animations.

@virajchandra51
Copy link
Contributor Author

For proper animation you can try to explore some library like react-spring. They usually help with these animations.

Sir I wanted to use Framer, or maybe this also works. But I did not know we can use libraries, I wanted to install react icons also but sir said to use pngs

@sahil9001
Copy link
Contributor

"Load More" functionality is correct but when "Show less" is pressed it isn't cascading smoothly.

@virajchandra51
Copy link
Contributor Author

@sahil9001 @prakhar-asaiya
Added the required animation of fadein//out in problem section
To remove some console log syntax errors (futile changes but seemed correct to change for better code), I added the
" key = {index} " statement in section 3,4,6,7. This removes the errors that were popping in the console as warnings by react. No logic changes.

Please review the animations, (only fadeIn works) and tell as required.

@sahil9001
Copy link
Contributor

sahil9001 commented Jan 5, 2023

This answer describes what I'm exactly pointing at https://stackoverflow.com/a/33931219 . I think this should be achievable by using the mentioned css in the answer

Comment on lines 46 to 49
padding-bottom: 0;
box-sizing: border-box;
font-family: "Poppins", sans-serif;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

All mentioned properties are redundant. Already covered in the base css.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed

Comment on lines 2 to 9
display: flex;
align-items: center;
justify-content: center;
flex-direction: row;
flex-wrap: wrap;
gap: 4rem;
width: 100%;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Use display : grid instead of flex, with grid-template-columns: repeat(2 ,1fr) for keeping 2 items per row.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done


.codeutsava__section8-image {
transition: all 0.2s ease-in-out;
width: 100%;
Copy link
Contributor

Choose a reason for hiding this comment

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

Add image height as 20rem

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@virajchandra51
Copy link
Contributor Author

Added the fadeOut animation.

@sahil9001
Copy link
Contributor

Impressed by your work! Keep it up!

@sahil9001 sahil9001 merged commit 70aa3f9 into TCP-Tech:dev Jan 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants