Abdihafid Adan
@Abdul400All comments
- @Abdul400@Abdul400
hey @KaiPereira
thank you for your feedback. I will be considering them for my next challenge. :)
- @Eman-AbdElZaher@Abdul400
hello @Eman-AbdElZaher, Generally, your work looks really good and is very responsive. However, here are a few things I noticed.
- the link to your GitHub code does not work and there is no way of reviewing the code.
- I bypassed that by using the view page source in chrome web dev tools and noticed that you gave the hero-desktop image a width and height of 100% which squishes it when viewing devices with smaller width. I would recommend using the image as a background of the right-side and then positioning it accordingly using
background-position
. - The form placeholder should also be the same color as that of the paragraph. You can edit the placeholder using
::placeholder
property.
Other than that. Good Job!!
- @AnulQr@Abdul400
hello @AnulQr. I think your work generally looks really good particularly for the desktop view. However, I noticed a few things.
- It is responsive but the pages have not been optimized well for mobile devices and tablets.
- The responsive layout transitions to quickly to the mobile view too quickly and according to your code, I can see that you started with mobile-first development, which is not necessarily a bad thing. However, you have to consider other devices too. I believe the transition from desktop view to mobile view should be from around 375px, but I usually start around 500px to cater for 'phablets' because phone screens are getting bigger. There should also be tablet devices maybe starting from 500px to around 800px or so. Then the rest is desktop view. kindly consider such devices in your development.
- The background does not match the one outline in the design files. consider incorporating it and adjusting it accordingly.
Other than that, Great Job!
Marked as helpful - @Mhmd-Tarek-Mhmd@Abdul400
hello @Mhmd-Tarek-Mhmd,
I noticed that the JavaScript functionality for email validation works only when the email is invalid. as per the design files, I believe you should also have a (green text) notification when the email is valid. I also noticed that the background positioning is a bit off. Use the
background-position
property to position your background.Other than that, excellent work!
Marked as helpful - @sqle157@Abdul400
hi, @sqle157.
Generally, everything looks good and scalable. However, I think you have to work on the hover states for instance, use
cursor: pointer
on the hover states for the avatar image, and also the name in the NFT component. Also, to allow for accessibility, kindly wrap the primary contents in a main html attribute. On the same note, provide alt descriptions for your images to also allow for accessibility.Otherwise, nice Job for having a clean and readable code!
Marked as helpful - @Abdul400@Abdul400
will definitely do so. Thank you for sharing your resources. I will definitely look into them.
- P@brodiewebdt@Abdul400
I'd really like to know what your web dev setup looks like. I like how you develop so quickly and they still look so good.
- P@brodiewebdt@Abdul400
I'm not sure how you develop your solutions so rapidly, I see you uploading in quick successions. Do you use bootstrap?
- @Abdul400@Abdul400
Thank you so much, this solved my issue. I thought making the color above the picture and giving it a lower opacity was the only way to do so. I'm still not sure how your solution works, but it works. Could you please explain to me? unless the bg-intro-desktop.png itself has a transparent background, I'm not sure how the hsl(0, 100%, 74%) can be seen.
- P@brodiewebdt@Abdul400
I have to admit this looks awesome!!
- @Abdul400@Abdul400
That solved my issue. Thank you!!
- @shashankbhat2@Abdul400
hello @shashankbhat2, your solutions looks great and the hover states are exactly as depicted. However, there are some things that I noted:
- clear your accessibility report.
- the page is not responsive for mobile i.e., 375px. I think you should work on making it responsive for mobile devices.
- the avatar does not have a white outline by default. It should have an outline on hover. So add a hover state to the image and give it a white outline.
Marked as helpful - @estivenson94@Abdul400
Hello @estivenson94. I think the solution looks really great and you've tried a lot. At a higher resolution, everything looks good. However, I found an issue when scaling down to 500px and I noticed that's where you used your media query. The issue I found is that the music icon and the annual plan are too close. I would recommend that you use a greater margin between the music icon and the annual plan. So give the music icon a
margin-right
of about20px
and everything will look good again. - @amakaogujiofor@Abdul400
hello @amakaogujiofor. Nice trial on the project. Here are some things that I noted.
- I've had some issues with the responsiveness especially at around 700px and I noticed that is where you used your media queries.
- Do not use
margin: auto
on the body element. - I would encourage you to set the body element as flexbox and center your card using
justify-content: center
, andalign-items: center
. Your card will always be centered and you can use this as the basis for making it responsive. - To position your background, use background position property to position it in the x and y axis. Here is a short and concise resource on the background position property on w3Schools
- P@brodiewebdt@Abdul400
No, I don't think that is an issues because I did the same. All my current front-end mentor projects are put in one public repository and I think everyone can clone or download it.
- P@brodiewebdt@Abdul400
This looks awesome! I wish you could make your repository public so that I can look at your code as well.
Marked as helpful - P@brodiewebdt@Abdul400
hello @brodiewebdt. I think generally the design looks good. However, I think the scale is off. The contents should span to the whole page i.e. it should be full screen and you have shown only a percentage of it (I think you have seen my example). Also I've seen that you have not incorporated tablet layout for your website as the mobile layout starts off too early at 800px. I'd recommend having a desktop layout, a tablet layout, and a mobile layout. I wanted to check out your code but for some reason, I've been unable to clone your code on GitHub. I think you should make your repositories public so that I can clone or download it.
Marked as helpful - @Abdul400@Abdul400
hello David. Thank you for your feedback. I'm not sure why the page is not responsive on your end. Are you viewing the website on a mobile phone or computer? what is your resolution. On my end, I've tested on every devise and it is very responsive. Is it possible for you to provide like a screenshot to show where it starts breaking? please use https://snipboard.io/ and send a link here. Thanks