@FrederikDiamonDSubmitted over 2 years ago
I made this using SCSS.
Please come with feedback. There's most likely something I could have done differently :)
Papa Elhadj Abdoulaye NDOYE
@NDOY3M4NAll comments
- @NDOY3M4NPosted over 2 years ago
Hi there. Great work on this challenge. I think you used the features (nesting mostly) correctly. Now there are some things in the design that you forgot.
On mobile, because of the
height: 100vhon themainthe content is not showing properly. To fix this issue, you could replace that property withmin-height: 100vhso that themainwill only fill up the minimum space needed.You could also add some vertical padding on mobile.
Happy coding ποΈ
Marked as helpful1 - @wyliemickelsonSubmitted over 2 years ago@NDOY3M4NPosted over 2 years ago
Wonderful ποΈποΈποΈ.
You just need to fix:
- on
1366x750screen, the modal is not showing properly (the close button is off the page), you could reduce the size of the carousel to accommodate for that. - on desktop, when hovering on the links the whole navbar is slightly moving. You could add the border with a transparent color on the default state, when in hover state you just change the color of the border.
- you could also allow the users to open the navbar on mobile
Happy coding ποΈ
0 - on
- @uspazSubmitted over 2 years ago
Buenas, cualquier comentario que pueda ayudarme a mejorar tanto este, como otros desafΓos es bienvenido. Saludos!
@NDOY3M4NPosted over 2 years agoNice job on this challenge. On mobile you could replace the
height: 100vhtomin-height: 100vhon thecontainerclass. You could also add some vertical padding on mobile. Other than that I think everything is okay. @Lucas feedback is also great. I use a similar technique when doing this challenge. Happy coding ποΈMarked as helpful1 - @mcdulingmSubmitted over 2 years ago
Trying to do these challenges as quickly as I can when I have time. Any feedback will be much appreciated. :)
@NDOY3M4NPosted over 2 years agoHi, great work on this challenge. There are just some things that need fixing like:
- on mobile the padding is not applied to the screen since you set
height: 100vhon the main. You could fix this by just replacing it withmin-height: 100vh. - on mobile you could also increase the padding (like in the design)
- on line 37 (
index.html) there is a typo, the closing tag should beainstead ofbutton.
Happy coding ποΈ
Marked as helpful1 - on mobile the padding is not applied to the screen since you set
- @MJ1001Submitted over 3 years ago
Is there anything I can improve upon?
@NDOY3M4NPosted over 3 years agoYou could add a
max-widthto thecontainerclass to sort-of constraint the width on mobile display. Great work1 - @imjackfrost1997Submitted over 3 years ago
Testing my HTML5 CSS3 skills
@NDOY3M4NPosted over 3 years agocongrats, that's a super nice reproduction of the design. Keep at it!
0 - @NDOY3M4NSubmitted almost 4 years ago
Hi, everyone! I learned a lot with this challenge by using some web API like MutationObserver and modularize my javascript. If you have any feedback (specially on my BEM naming convention or on how to help me minimize my CSS), feel free to let me know.
@NDOY3M4NPosted almost 4 years agoOups, you're right. I totally forgot to change the hover styles on mobile. Thanks for the feedback
1 - @Andro87Submitted about 4 years ago
Hello! Any feedback is welcome ! ;)
@NDOY3M4NPosted about 4 years agoHi @Andro87, nice reproduction of the design. Maybe you can try adding the hover effect on the cards to complete the challenge.
0 - @fatihcandevSubmitted about 4 years ago
You can see the slider codes in the utils folder in GitHub repo. It is pretty easy with only JS and CSS.
@NDOY3M4NPosted about 4 years agoYay @fatihcaen congratulations on your implementation of the slider. It looks super smooth. One thing I might suggest is to add an
overflow: hiddenwhen the mobile navbar is open to prevent vertical scrolling.Happy coding :smile:
Marked as helpful1 - @N1612NSubmitted over 4 years ago@NDOY3M4NPosted over 4 years ago
Nice reproduction of the design. For the mobile view, you could set the max-width to 500px or 700px instead of 300px to cover the range of most of the mobile devices
0 - @madizhaksylykSubmitted over 4 years ago
Hi everyone! I worked on atom and the final code looked just fine. But now the last box is not where it should be. I know, my code seems a bit long and messy, so I want your tips.
@NDOY3M4NPosted over 4 years agoHi, nice approach with the positioning but there is a lot of repetition in your CSS. Also it seems like you forgot the mobile approach. You could start with the mobile approach and then adapt your code for the desktop view.
1 - @DevcrowmasterSubmitted over 4 years ago@NDOY3M4NPosted over 4 years ago
Yo! You did a great job at following the designs in the mobile and desktop layout. Bravo! In your desktop layout, maybe you should set the background-size to cover in desktop and 100% in mobile
1
