Skip to content
OverviewSolutions
25
Comments
75
Marzuk Sanni
@Zukizuk

All comments

  • Marzuk Sanni•1,360
    @Zukizuk

    Hello 👋. Your solution looks amazing, as always.

    I am done with this project but I was wondering how to sort the initial fetch result to match that of the design. Can you please tell me the criteria you are using for the sort ?

  • Marzuk Sanni•1,360
    @Zukizuk

    FEEDBACK TEMPLATES

    On semantic HTML

    Hello there, Congratulations on completing your first challenge here!

    Your solution looks nice

    I have a few suggestions that I think might be of interest to you.

    - HTML: Consider using semantic HTML tags like <main></main>,<section></section> and others that you can find in this link. The main tag should hold all the main content of you page so note that.

    The semantic HTML tags help search engines and other user devices determine the importance and context of web pages. The pages made with semantic elements are much easier to read. It has greater accessibility. It offers a better user experience. Using div when there's a better alternative is not a good practice as div holds no semantic value.

    The html styling is not styled like this

    html{
    background-color: hsl(35, 94%, 93%);
    border-radius: 2%;
    max-width: 95%;
    padding: 2.5%;
    margin-left: auto;
    text-justify: auto;
    word-spacing: auto;
}

    You gave the body a fixed with which shouldn't be so body{width: 1440px}. These stylings also need to be placed at the topmost of your styles not the bottom.

    @font-face {
    font-family: YoungSerif-Regular;
    src: url(./assets/fonts/young-serif/YoungSerif-Regular.ttf);
}
@font-face {
font-family: Outfit-VariableFont;
    src: url(./assets/fonts/outfit/Outfit-VariableFont_wght.ttf)
    }

    You html also need more work on it concerning semantic html, naming of html file, and more and I would like to correct you on all that but the feedback would be too long to do, so suggest you check my solution for this challenge here /my-solution. And if you don't get anything, you can write under it and I would explain to you.

    I hope this feedback is helpful.

    Other than that great job

    Marked as helpful
  • Marzuk Sanni•1,360
    @Zukizuk

    Hello there, and congratulations on completing this challenge.

    Your solution looks nice.

    I have a few suggestions that I think might interest you.

    Your use of semantic HTML was done well, but there's something that you should know about the use of section. Whenever you use a section, you are required to provide a heading for that section. Your <section class="content" has a heading, but the other one doesn't. In this situation, I think you should have used div, since all the content on this challenge is related and on the same page too. Use sections when the content is not related; otherwise, provide a heading for each section.

    I hope this feedback is helpful.

    Other than that, good job!

    Marked as helpful
  • Marzuk Sanni•1,360
    @Zukizuk

    Hello there, Congratulations on completing this project!

    Your solution looks nice

    I have a few suggestions that I think might be of interest to you.

    - HTML: Consider using semantic HTML tags like <main></main>,<section></section> and others that you can find in this link. And in this case, replace <div class="container"> with <main class="container">. The main tag should hold the main content of the page so consider correcting that.

    The semantic HTML tags help search engines and other user devices determine the importance and context of web pages. The pages made with semantic elements are much easier to read. It has greater accessibility. It offers a better user experience. Using div when there's a better alternative is not a good practice as div holds no semantic value.

    CSS: Also, you need to reset your style sheet at the start of your webpage before you start actually styling your webpage. Like this

    * {
margin: 0;
padding: 0;
box-sizing: border-box;
}

    Consider using the above code at the start of your CSS file to reset the stylings for you before you begin.

    I hope this feedback is helpful.

    Other than that great job

  • Marzuk Sanni•1,360
    @Zukizuk

    Hello there, Congratulations on completing this project

    Your solution looks nice

    I have few suggestions that I think might be of interest to you.

    - HTML: Consider using semantic HTML tags like <main></main>,<section></section> and others that you can find in this link. And in this case, replace the <div class="main"> with <main class="main">. the main tag should hold the main content of the page so consider correcting that.

    The semantic HTML tags help the search engines and other user devices to determine the importance and context of web pages. The pages made with semantic elements are much easier to read. It has greater accessibility. It offers a better user experience. Using div when there's a better alternative is not a good practice as div hold no semantic value.

    - CSS: use the following stylings to help center your card

    body {
min-height: 100vh;
display: grid;
place-items: center;
}

    I hope this feedback is helpful

    Other than that great job

  • Marzuk Sanni•1,360
    @Zukizuk

    Hello there, Congratulations on completing this project

    Your solution looks nice

    I have few suggestions that I think might be of interest to you.

    - HTML: Consider using semantic HTML tags like <main></main>,<section></section> and others that you can find in this link. And in this case, replace the <div class="container"> with <main class="container">. the main tag should hold the main content of the page so consider correcting that.

    The semantic HTML tags help the search engines and other user devices to determine the importance and context of web pages. The pages made with semantic elements are much easier to read. It has greater accessibility. It offers a better user experience. Using div when there's a better alternative is not a good practice as div hold no semantic value.

    The heading elements are H1, H2, H3, H4, H5, and H6 with H1 being the highest (or most important) level and H6 the least. Only use one <h1> per page - this should represent the main heading/subject for the whole page. Also, do not skip heading levels - start with <h1>, then use <h2>, and so on. The heading elements are H1, H2, H3, H4, H5, and H6 with H1 being the highest (or most important) level and H6 the least. In your code I didn't see any h1 tag but h2 which is wrong so consider fixing that.

    I hope this feedback is helpful

    Other than that great job

  • Marzuk Sanni•1,360
    @Zukizuk

    Hello there, Congratulations on completing this project

    Your solution looks nice

    I have few suggestions that I think might be of interest to you.

    - HTML: Consider using semantic HTML tags like <main></main>,<section></section> and others that you can find in this link. And in this case, replace the <div class="container"> with <main class="container">. the main tag should hold the main content of the page so consider correcting that.

    The semantic HTML tags help the search engines and other user devices to determine the importance and context of web pages. The pages made with semantic elements are much easier to read. It has greater accessibility. It offers a better user experience. Using div when there's a better alternative is not a good practice as div hold no semantic value.

    The heading elements are H1, H2, H3, H4, H5, and H6 with H1 being the highest (or most important) level and H6 the least. Only use one <h1> per page - this should represent the main heading/subject for the whole page. Also, do not skip heading levels - start with <h1>, then use <h2>, and so on. The heading elements are H1, H2, H3, H4, H5, and H6 with H1 being the highest (or most important) level and H6 the least. In your code I didn't see any h1 tag but h3 which is wrong so consider fixing that. Please note that if there's no h1. there shouldn't be an h2 or h3. But again every page need an h1.

    Also if you want to center your card. You could have used align-items: center; since you used display: flex; to center the card vertically

    I hope this feedback is helpful

    Other than that great job

  • Marzuk Sanni•1,360
    @Zukizuk

    Hello there, Congratulations on completing this project

    Your solution looks nice

    I have few suggestions that I think might be of interest to you.

    - HTML: Consider using semantic HTML tags like <main></main>,<section></section> and others that you can find in this link. And in this case, replace the <div class="main"> with <main class="main">. the main tag should hold the main content of the page so consider correcting that.

    The semantic HTML tags help the search engines and other user devices to determine the importance and context of web pages. The pages made with semantic elements are much easier to read. It has greater accessibility. It offers a better user experience. Using div when there's a better alternative is not a good practice as div hold no semantic value.

    Also you didn't include an H1 tag in your page. Note that every page need an H1 tag and you don't have to skip them. I mean if you should have an h1 before h2 and so on. So please fix that.

    I hope this feedback is helpful

    Other than that great job

    Marked as helpful
  • Marzuk Sanni•1,360
    @Zukizuk

    Hello there, Congratulations on completing this project

    Your solution looks nice

    I have few suggestions that I think might be of interest to you.

    - HTML: Consider using semantic HTML tags like <main></main>,<section></section> and others that you can find in this link. And in this case, replace the <div class="card"> with <main class="card">. the main tag should hold the main content of the page so consider correcting that.

    The semantic HTML tags help the search engines and other user devices to determine the importance and context of web pages. The pages made with semantic elements are much easier to read. It has greater accessibility. It offers a better user experience. Using div when there's a better alternative is not a good practice as div hold no semantic value.

    The heading elements are H1, H2, H3, H4, H5, and H6 with H1 being the highest (or most important) level and H6 the least. Only use one <h1> per page - this should represent the main heading/subject for the whole page. Also, do not skip heading levels - start with <h1>, then use <h2>, and so on. The heading elements are H1, H2, H3, H4, H5, and H6 with H1 being the highest (or most important) level and H6 the least. In your code I didn't see any h1 tag but h2 which is wrong so consider fixing that.

    I hope this feedback is helpful

    Other than that great job

  • Marzuk Sanni•1,360
    @Zukizuk

    Hello there, Congratulations on completing this project

    Your solution looks nice

    I have few suggestions that I think might be of interest to you.

    - HTML: Consider using semantic HTML tags like <main></main>,<section></section> and others that you can find in this link. And in this case, replace the <div class="main"> with <main class="main">. the main tag should hold the main content of the page so consider correcting that.

    The semantic HTML tags help the search engines and other user devices to determine the importance and context of web pages. The pages made with semantic elements are much easier to read. It has greater accessibility. It offers a better user experience. Using div when there's a better alternative is not a good practice as div hold no semantic value.

    The heading elements are H1, H2, H3, H4, H5, and H6 with H1 being the highest (or most important) level and H6 the least. Only use one <h1> per page - this should represent the main heading/subject for the whole page. Also, do not skip heading levels - start with <h1>, then use <h2>, and so on. In your code, I didn't see any heading element. and as I said. every page need at least h1 tag so consider fixing.

    I hope this feedback is helpful

    Other than that great job

    Marked as helpful
  • Marzuk Sanni•1,360
    @Zukizuk

    Hello there congratulation on completing this challenge.

    Your solution looks nice.

    I have few suggestion that I think might interest you.

    You shouldn't use section just to divide things, in such situations use a div element instead like. The <section> tag in HTML is used to define sections in a document. It's typically used to group related content together, such as chapters, headings, or any thematic grouping of content. Also every section tag needs at least one heading tag like h1 or h2 or h3 so think about it before using it.

    In this particular challenge you use section tag to group the element which I think wasn't best. You should have used a div element because then you won't have to provide any heading tag and hide it.

    I hope this feedback is helpful

    Other than that great job

    Marked as helpful
  • Marzuk Sanni•1,360
    @Zukizuk

    Hello there, Congratulations on completing this project

    Your solution looks nice

    I have few suggestions that I think might be of interest to you.

    I think you shouldn't have used a section here <section class="image"> instead use div. Using a section requires you to provide a heading tag in the section. you can read more about that here heading element.

    I hope this feedback is helpful

    Other than that great job

    Marked as helpful
  • Marzuk Sanni•1,360
    @Zukizuk

    Hello there, Congratulations on completing this project

    Your solution looks nice

    I have few suggestions that I think might be of interest to you.

    The heading elements are H1, H2, H3, H4, H5, and H6 with H1 being the highest (or most important) level and H6 the least. Only use one <h1> per page - this should represent the main heading/subject for the whole page. Also, do not skip heading levels - start with <h1>, then use <h2>, and so on. The heading elements are H1, H2, H3, H4, H5, and H6 with H1 being the highest (or most important) level and H6 the least. In your code I didn't see any h1 tag but h3 which is wrong so consider fixing that.

    In this challenge for example, there's no text that can serve as an appropriate content for h1 tag so in this situation you do something like this<h1 class="sr-only">3-column-preview-card-component</h1> and in the css, you hide the h1 with the help of position: absolute; to hide it from view. The H1 is needed for accessibility and seo so you need it in every page. You can read more about that here heading element.

    If there's no text in your page to serve as the h1 tag, provide one yourself which can best serve as subject/ main heading of the page and hide it with position: absolute;

    I hope this feedback is helpful

    Other than that great job

  • Marzuk Sanni•1,360
    @Zukizuk

    Hello there, Congratulations on completing this project

    Your solution looks nice

    I have few suggestions that I think might be of interest to you.

    According to the HTML specification, a page should only have one <main> element. The <main> element represents the main content of the document, and it's intended to contain the primary content that is directly related to or expands upon the central topic of the document. You have two main tags in your html code so please consider fixing that. Also you page didn't have a h1 tag and every page need an h1 tag as the main subject or main heading. You can read more about them here heading element.

    I hope this feedback is helpful

    Other than that great job

    Marked as helpful
  • Marzuk Sanni•1,360
    @Zukizuk

    Hello there, congratulation on completing this challenge

    Your solution look nice.

    I have few things to suggest that I think might interest you.

    Consider using anchor tag <a href=""></a> for the links instead of buttons. Even when you are not providing links for them, use anchor tag for what is meant to be links and style it with css to make it look like a button.

    Also your page had no h1 tag. Note that every page need to have at least a heading tag and that should be <h1></h1> tag. You need one of this per page. Please note that. You can read more about them here heading tags

    I hope this feedback is helpful.

    Other than that you did a great job

    Marked as helpful
  • Marzuk Sanni•1,360
    @Zukizuk

    Hello there, Congratulations on completing this project

    Your solution looks nice

    I have few suggestions that I think might be of interest to you.

    - HTML: Consider using semantic HTML tags like <main></main>,<section></section> and others that you can find in this link. And in this case, replace the <div class="container"> with <main class="container">. the main tag should hold the main content of the page so consider correcting that.

    The semantic HTML tags help the search engines and other user devices to determine the importance and context of web pages. The pages made with semantic elements are much easier to read. It has greater accessibility. It offers a better user experience. Using div when there's a better alternative is not a good practice as div hold no semantic value.

    I hope this feedback is helpful

    Other than that great job

  • Marzuk Sanni•1,360
    @Zukizuk

    Hello there, Congratulations on completing this project

    Your solution looks nice

    I have few suggestions that I think might be of interest to you.

    - HTML: Consider using semantic HTML tags like <main></main>,<section></section> and others that you can find in this link. And in this case, replace the <div> with <main>. the main tag should hold the main content of the page so consider correcting that.

    The semantic HTML tags help the search engines and other user devices to determine the importance and context of web pages. The pages made with semantic elements are much easier to read. It has greater accessibility. It offers a better user experience. Using div when there's a better alternative is not a good practice as div hold no semantic value.

    Also you didn't use <a href=""></a> for the links, whenever you have a link in a page make sure you use anchor tag. So in this situation you should put the a tag in the ul li tag like this.

    <ul>
      <li><a href="">Github</a>
</ul>

    Also I didn't see you use <h1> tag in your page, you should note that every page should contain <h1></h1> and there should be only one of it. It can't be two so note that

    I hope this feedback is helpful

    Other than that great job

  • Marzuk Sanni•1,360
    @Zukizuk

    Hello there, Congratulations on completing this project

    Your solution looks nice

    I have few suggestions that I think might be of interest to you.

    - HTML: Consider using semantic HTML tags like <main></main>,<section></section> and others that you can find in this link. And in this case, replace the <section class="card"> with <main class="card">. the main tag should hold the main content of the page so consider correcting that.

    The semantic HTML tags help the search engines and other user devices to determine the importance and context of web pages. The pages made with semantic elements are much easier to read. It has greater accessibility.

    Also I think your use of sections here is a little bit much, though we say don't use div when you don't have to because they don't have any semantic value, but in this case using div seem perfectly fine to me because this single card does have a maximum of two section which will be the rating and the thank you sections but I saw that you used section to separate the buttons and text on the card which I think you should have used div here. They are all on the same section. Use section when the content are not related. You can read more about it from the link I provided.

    I hope this feedback is helpful

    Other than that great job

    Marked as helpful