Completely new to this world of web development (only learnt about Flexbox today) and having a look at some of the other solutions that people have submitted, realise how far I've got to go. Anyway, feel free to pull it apart and tell me where I've gone wrong, etc, all feedback welcome!!
Aliyu-Saidu
@Aliyu-SaiduAll comments
- @splwdevSubmitted almost 2 years ago@Aliyu-SaiduPosted almost 2 years ago
Hi Shane Lynch, you have done a very nice job. Keep it up.
Here are some suggestions to help you improve.
-
For proper outline structure of your website, h2 should come only after h1 and h3 should only come after h2 and so on. So you may consider replacing the h2 (or h3) you used to h1. Don't worry about the default browser font size, you can always change that with css depending on your design. Read more about accessibility here (https://web.dev/accessible/?gclid=CjwKCAiA7IGcBhA8EiwAFfUDsfX-xPDXrqqo7B0vZV44epOKlmpslZ4tepkhgjL7p6-gSKHmG7Li8RoC1_MQAvD_BwE). Or read more from the links provided above (accessibility reports).
-
Your design doesn't look good on mobile. You may wish to padding say 30px on the 'body' and remove the (margin: 30px 0) you set on the 'body'. This will give an all-round space between the screen the your design on mobile devices.
-
Also set specific height for your main Flexbox container (in this case, the body) to say 100vh. This will help to properly centralise your project on the page(vertically and horizontally).
-
If have more questions, feel free to contact me.
Hope this is helpful? You may consider giving me an upvote and hitting the helpful button bellow.
Happy Coding!
Marked as helpful1 -
- @nilasishroySubmitted almost 2 years ago@Aliyu-SaiduPosted almost 2 years ago
Congratulations, Nilasish Roy. I really like your solution. Find below some tips to improve your code!
-
Every Webpage should be contained in a landmark. In this case, you may use <main> and <footer> in place of <div class="project" > and <div class="attribution" > respectively, to improve the accessibility of your website. You can read more on accessibility on (https://web.dev/accessible/?gclid=CjwKCAiA7IGcBhA8EiwAFfUDsfX-xPDXrqqo7B0vZV44epOKlmpslZ4tepkhgjL7p6-gSKHmG7Li8RoC1_MQAvD_BwE).
-
You may also want to check the accessibility report links above too to read more on the subject.
-
You place the project vertically and horizontally in the center of the page, you could use css Flexbox (https://www.w3schools.com/css/css3_flexbox.asp) or Grid (https://www.w3schools.com/css/css_grid.asp).
-
If you have any specific questions, feel free to contact me. Hope this is helpful? Please if you like the feedback, consider hitting the helpful button and also give me an upvote. Thank you!
0 -
- @rpajaronSubmitted almost 2 years ago
what is the better way to create this project?
@Aliyu-SaiduPosted almost 2 years agoNice job there, Jaqen. Keep it up. Here are some tips to help improve your code.
- Every website must have at least one landmark such as <main>, <header>, <footer> and so on, for improve accessibility. So you may consider wrapping your entire component by <main> by changing your <div class="main" > to just <main>.
- Every website should also have a level one heading (h1). You may want to replace the first paragraph <p class="first" > to <h1>.
- Since you are already wrapping entire website by <div class="main" > container and another <div class="container" >, you should consider removing all the specifications on 'body' element and place it inside the <div class="main" > and re-evaluate your code. I believe this should help place the design properly in the center (both vertically and horizontally). I hope this help? Happy coding
Marked as helpful1 - @Ripra87Submitted almost 2 years ago
Hey guys, thanks for watching my project and for any suggestion! I have one main question, i couldn't use flex display to perfectly center the image vertically (align content: center, even if i added it) because after checking i saw that the body didn't cover the entire page, so the image was basically centered because the body height was exacly like the main height.. i had to add a padding to the body bottom and vertically move the rest down with the top margin.. i tried to add 100% height and width to the body without results.. anybody knows how to fix it? thanks everybody in advance, and happy coding!
@Aliyu-SaiduPosted almost 2 years agoNice work @Ripra87. Here are some suggestions to improve your code.
- To answer your questions, consider setting the width and height of the 'body' element to say 100vw and 100vh respectively. Then remove the padding and margin hack you used above. Please always avoid setting your 'body' element height in percentage. This may give you unwanted results as you saw above.
- For proper outline of your documents or website. Always start the first heading in your webpage with a level one (h1) element. Yes this challenge may just be (in real sense) a component of your website but as this is the only thing you have on your webpage, you should consider using h1 instead of the h3 you used in your code. This should solve the accessibility report warning you had above.
If you need more help or want to discuss this further, please feel free to contact.
Hope this helps?
If yes, you may consider hitting the helpful button below. Thanks!
Marked as helpful0 - @munzerxSubmitted almost 2 years ago@Aliyu-SaiduPosted almost 2 years ago
Nice work Munzer Mardini and congratulations for the successful completion of this challenge. Please find below tips to help you improve your code.
-Every website must have at least one landmark (like main, nav, footer). In this case, you may want to replace the div container with 'main'.
- To give proper space between your design and screen on mobile view, you may consider giving your body element a padding say 2rem. You could otherwise give margin to of say 2rem to the div container (which you could replace with 'main' container).
- You may consider using flex or grid to easily handle any layout on your website. It's easier and much more convenient. You can can read through that in w3schools.com.
- You may visit the accessibility report links provided for you above to learn more about accessibility.
Please feel free to contact me if you need more help.
If you find this feedback helpful, you may consider hitting the helpful button below.
Happy coding!
Marked as helpful1 - @BO-FirasSubmitted almost 2 years ago@Aliyu-SaiduPosted almost 2 years ago
Hi Mohamed Firas Ben Othmen, Congratulations for successful completion of this challenge. Here are some tips which I feel will help you improve your code.
- your design doesn't look good on desktop (not been properly centralised both vertically and horizontally). This is because you did not specifically state the height of the section container (with a class of d-flex). Setting the flex property (align-items: center) when the flex direction is row, requires stating the height of the flex container to be more than the overall content height. Otherwise, the effect will not be felt. So you may consider setting the height of the flex container (here in this case section with a class of d-flex) to be 100vh. This should solve the problem.
- To increase the accessibility of your website, you may wish to visit the accessibility report links provided for you above. It is self explanatory. But for this challenge, you could just replace the 'section' element with 'main' element. Hope this help.
Please click the helpful button below if you find this feedback helpful. Thanks.
If you also wish to discuss this further, I am always available and more than happy to help.
Marked as helpful0 - @GodwindanSubmitted almost 2 years ago
Guy I review on this and any corrections or suggestions.
@Aliyu-SaiduPosted almost 2 years agoHi @Godwindan and congratulations for completing this challenge. Here are some suggestions which I believe will help improve your solution.
- You may need to create 'main' element selector in your style.css file and set the width: 360px;
- Then remove the declaration width: 360px; on the <body> element selector. That should solve the desktop centralization problem.
If you have any questions, you are always free to contact me and I will be happy to help. If you find this comment helpful, please do hit the helpful button.
Happy Coding!
0 - @TheOliveraSubmitted almost 2 years ago@Aliyu-SaiduPosted almost 2 years ago
Hi @TheOlivera, Welcome to FEM and Congratulations for submitting your first solution to the platform.
Here are some suggestions which I think will help you fix the problem at hand.
- First, you need to change the index.html file in your github repository to the proper one. You have used an index.html file made for 'Order Summary Card' challenge in place of the 'NTF Preview Card Component' challenge. Every other files appears to be okay. Doing this should fix the problem.
- So go to github, remove the wrong file, upload the correct file and hit commit and then refresh the page. Then visit the live Url to be sure everything is working properly.
- Then come to the platform here and re-generate report. Your Solution should display properly.
If you have any specific questions, feel free to contact me, I will be more than happy to help.
If you find this feedback comment helpful, please hit the helpful button below the comment.
Thanks and Happy Coding!
0 - @Mageshwari-BalaguruSubmitted almost 2 years ago@Aliyu-SaiduPosted almost 2 years ago
Great work, @Mageshwari-Balaguru!
However, these are some of the suggestions which might help improve your solution.
-
you may use <img src="image-qr-code.png" alt="image of QR code " >. This will solve the problem of QR image not displaying in the above solution. Also note that every <img> element must have an alt attribute even if its going to be empty.
-
There can never an h2 element in a page without an h1.
-
Every page must have at least one landmark. You can read through each of the suggested links in the 'learn more about our accessibility report' above.
If have more specific questions, please feel free to contact me.
If you feel my feedback report is helpful, please click the helpful button below.
Happy coding and Welcome to Frontend Coding Challenge! 👍
Marked as helpful0 -
- @hrazithSubmitted almost 2 years ago@Aliyu-SaiduPosted almost 2 years ago
Hi @hrazith, that is a nice work!
- But doing a small project like this using react/tailwind css, don't you think is an overkill? Except if you just want to try out your react/tailwind skills.
- I suggest you to try Vanilla CSS Flexbox or Grid or both to make it look good on both mobile and desktop devices. You may wish to try to read 'Complete Guide to CSS Flex and Grid by Shruti Balasa' (https://shrutibalasa.gumroad.com/l/css-flex-and-grid/BlackFriday22) . It is one of the best books out there on the topic
- All together, I think you need to work on the desktop view to look close to the given design. Good luck
0 - @benj0s85Submitted almost 2 years ago
Je ne suis pas sur du responsive ainsi que de la sauvegarde de mes fichiers dans le GitHub
@Aliyu-SaiduPosted almost 2 years agoGreat work @benj0s85. However, here are some tips to help you fix some of the problems at hand and to also help you improve in your subsequent challenge!
- looking at the your live site, it is like you have put a wrong link. Please consider reading how to submit your work this Frontend mentor website.
- another look at your codes, it was like you did two separate designs (mobile and desktop). This means you need read more on responsive design in w3schools (https://www.w3schools.com/html/html_responsive.asp) or use either CSS Flexbox or Grid.
- You could also check the solution of my work as a reference. If you have any questions or clarification, please feel free to contact me.
Marked as helpful0 - @obiajSubmitted almost 2 years ago
I had to rework on my solution after your reviews thhanks i will be expecting more feedbacks
@Aliyu-SaiduPosted almost 2 years agoHi @Obiaj, that is a great work! But
- first the mobile view doesn't look good on mobile device. you might want to check that out and fix it.
- You might also want to use semantic html5 Mark up to increase accessibility of your work. Consider checking w3schools to learn about that. You could also check the accessibility links provided above for more information about that.
Marked as helpful0