@SzymonRojek
Posted
Hi Okoroji,
Well done :D
I have checked your HTML (mainly), a few tips from me:
- this div class="wrapper" is unnecessary, use the header tag;
- in the header you can create the h1 with two spans inside (main-heading: Reliable, efficient delivery and sub-heading:Powered by Technology);
- instead of section you can use the main tag just to indicate the main content of this page and inside these four divs => they are perfect for generic groupings of content;
- alt text => In this project, images have an only a decorative role - that's a reason why alt text should be provided as an empty (alt="") so img can be ignored by assistive technologies, such as screen readers;
- names of the classes should be readable and descriptive => maybe start to learn the BEM naming convention;
- check the design folder and create spacing between elements => padding / margin;
- add align-items center to the header (check mobile size);
- max-width should be max-width: 1440px (your version is 1000 without px);
- also you can create RWD dedicated for tablets => one column for mobiles, then two rows with two boxes each, and finally desktop design pattern;
- repository: create separate folders for CSS, images because at the moment it is hard to navigate between different files.
Ps. Don't forget to upvote any comments on here that you find helpful.
Greetings :D
@vickymarz
Posted
I am really thankful for the corrections. Please, is the div class ="wrapper" unnecessary only in the header or in all the sections where it was applied?
@SzymonRojek
Posted
@vickymarz
I will check it later because I am not at home, thanks 🙏
@SzymonRojek
Posted
@vickymarz
Hi, I wanted to check your HTML again but you have deleted the files and now will be difficult to make a correct answer. Advice: in my opinion you don't have to delete them because all your history of commit also were deleted. Try to always work on the same file, keep the history with commits. That's quite important when the big project comes up :D
Have a nice day :D
@vickymarz
Posted
Thanks for that. You can now view the page now as I have made some adjustments based on your observations earlier. I will be looking forward ur reviews
@SzymonRojek
Posted
@vickymarz
Well done, much better right now:
- please, do not use br tag, only when you really need it but in this solution definitely we do not have to use it (read about it **=>**stack overflow will be useful);
- it is hard to read your HTML right now because of those wide blanks, spacing (use the tools dedicated to it with VSC);
- in my opinion you don't need these div class="wrapper" and also instead of section just use divs => as I mentioned before, they are perfect for generic groupings of content;
- check a project by using the inspector in your browser on different devices (have a look Galaxy S III, Galaxy Note 3 etc.) => add text align center to the header (below 360px approximately);
Please don't forget to upvote all my comments here because this is a kind of a small award for me.
Keep coding :D
@vickymarz
Posted
@SzymonRojek I really appreciate you for taking your time to put me through.
I always seems to use sections for different parts of my code and then use a div tag inside the section tag for styling purpose. I have read lots of blogs and materials but I can't seem to understand the how best or where to use section tag in my project. I will be glad if you can possibly put an end to my confusion.
Secondly I use wrapper inside my project cos I am not very conversant with bootstrap and I use the wrapper to adjust the width of a given content inside a div tag or a section tag
@SzymonRojek
Posted
@vickymarz
Sorry for my delay. I have used to use a section like you before and article too. I have received an information that we can use article elements for content that could stand alone by itself. For example: blog posts, news articles, forum posts. What's about section: this element we can use for larger groupings of related content. For example, the feedback section on the solution page.
In this project we could use an article for each box but in my opinion it is unnecessary => divs will be fine because they are perfect for generic groupings of content.
It is not easy to use semantic tags. I always think a lot before I apply them because it is very easy to overuse them.
Thanks for upvoting my comments.
Greetings :D
