Closed avikt18 closed 1 year ago
@Shivam-AfA @leecalcote Things left to be done:
🚀 Preview for commit 56f2d6fa7962f37ffac756263258e94eb9687bd5 at: https://640f0ad6fbc9f75f15b35e54--layer5.netlify.app
@avikt18 This looks nice. The way cards react to hover is pretty neat. You can use the text in the features section below the card carousel.
Suggestions received:
🚀 Preview for commit ca566f8f94f66e0a84abda268034452aca5c0694 at: https://641067067eb0860adb65d14f--layer5.netlify.app
🚀 Preview for commit 34af4b1125c1263f71ab0824be204a90183b42b1 at: https://64106db2b6333b06e3c78edc--layer5.netlify.app
Roll over effect 1.0 (Clicking on any card show the next card but doesn't change the order of them in the deck) https://641067067eb0860adb65d14f--layer5.netlify.app/cloud-native-management/meshmap/design
Roll over effect 2.0 (Clicking on any card show the next card and also places the current active card at last in deck) https://64106db2b6333b06e3c78edc--layer5.netlify.app/cloud-native-management/meshmap/design
@leecalcote @Shivam-AfA thoughts on the two variations?
@avikt18 I think the 2.0 effect looks better. Your thoughts @leecalcote? Although I think it would be better if we use the card with only the green gradient in the center. (Currently the one containing both blue and green gradient is being used).
@leecalcote @Shivam-AfA how about we auto play the carousel? Also, Shivam you are right, the green gradient would look nicer and have better readability
🚀 Preview for commit 9cf9c3670a62e9904baa136832111bf116f36fc6 at: https://64146365a0506315b9dd06a6--layer5.netlify.app
🚀 Preview for commit ada75d914e003eea196d0446a25db924e6583d56 at: https://64146d8e5e38511d1b473990--layer5.netlify.app
@avikt18 please fix these overflow issues.
And also I can still see the card with blue gradient being used in the center. It should be changed to green.
And about the autoplay, it's a good idea, although, it sometimes becomes irritable to the user when the user is trying to read something and it just moves away. So we can save that for later. Currently please make these changes and also correct the remaining responsiveness issues.
@avikt18 Let's discuss this on the Websites call. Please add this as an agenda item in the meeting minutes if you would. :)
🚀 Preview for commit fdd833996bfeba9e3d5c2e883b38136a2d2a0939 at: https://64183e9c97a24a3470ec150c--layer5.netlify.app
@Shivam-AfA there are few unreviewed commits here. Is this PR acceptable or needs more work?
@Shivam-AfA so do you want me to remove the heading completely on smaller screens or place it above the cards?
@avikt18 We can move the heading above the cards on all screen widths, so the heading can be used to commence the features section and the active card can be displayed to full width.
@Shivam-AfA We can make the heading above the cards only below 768px when the whole page becomes centre aligned otherwise the whole heading-in-left-image-in-right design would break. Above that, I guess it would look off.
Above 767px, we can separately center align the heading.
This is being done to provide a full width to the card, so that a supporting image could also be added inside it.
This is being done to provide a full width to the card, so that a supporting image could also be added inside it.
Ohh, got it
🚀 Preview for commit 0aedaf5415ce5dba89384d0f881342eb1a6d41b7 at: https://641ad6deca810605a5dabc95--layer5.netlify.app
@avikt18 Did you work on the feedback?
@Shivam-AfA I am working on it. Should I put a sample image in the card just like the mobile swiper version? Also, I have removed the floating animation as discussed in Monday meeting.
@avikt18 Yes sure, add a sample image.
🚀 Preview for commit 5a020b8339afa1d6ce3f69b81a73bde87a7b9681 at: https://641c69cf9ff8b607da81967d--layer5.netlify.app
@avikt18 Thanks for doing this. Some final touches:
@leecalcote Does this look good to merge?
🚀 Preview for commit 1cbb2c032befdeefb2d2445cea01e07b011ba74e at: https://641daee3b0ea48354b62ef15--layer5.netlify.app
@avikt18 This looks really good. We'll merge this, but need some changes before publishing this:
You can have another PR with these changes.
@avikt18 Let's discuss this on the Websites call. Please add this as an agenda item in the meeting minutes if you would. :)
🚀 Preview for commit 36592ab457d208abe5ab6d34d348f9ba1760474d at: https://64220049e95ed307f3b6941a--layer5.netlify.app
Description
This PR fixes #3866
Notes for Reviewers
Signed commits