Skip to content
OverviewSolutions
11
Comments
10
P
schindlerdumagat
@schindlerdumagat

All comments

  • Vincin Christmas•170
    @VincinChristmasSubmitted 10 days ago
    What are you most proud of, and what would you do differently next time?

    I am happy that I am getting more familiar with Sass and JavaScript event listeners

    What challenges did you encounter, and how did you overcome them?

    the JavaScript portion was a little challenging.

    What specific areas of your project would you like help with?

    I know that I am not supposed to use styling in the JavaScript, so I will need help with how to change that. I plan to come back and fix this later.

    The News Letter Sign Up

    #sass/scss
    1
    P
    schindlerdumagat•250
    @schindlerdumagatPosted 5 days ago

    Here are the issues I saw:

    HTML

    • You should enclose all the html code in the body element within a landmark role. In this case, you can put it in a main tag.
    • I recommend that you use a picture element for the illustration image to be able to download only what is needed based on the device width. Your solution currently download all of the illustration even though you only need the mobile version if you are on mobile view for example.
    • I recommend that you use h2 tag instead of h1 for the form title because the form will only be a part of the page and it will not probably hold the main title of the page.
    • The icon list are just decorative. They don't provide context to the page so an alt attribute with the value of empty string is enough to make it decorative. Do the same thing with the icon image in the success page.

    CSS

    • There are still some inconsistencies with the UI. It seems like the tablet view was not implemented. You also need to fix some parts of UI to closely match the design provided.

    JS

    • As you have mentioned, you have added styling logic to your JavaScript code directly. For you to fix that, you can create classes instead and style them in your CSS. To apply those classes in your elements, you can use methods on your elements such as element.classList.add("className"), element.classList.remove("className"), element.classList.toggle("className") just to name a few. You can learn more about how this code works on the internet.
    Marked as helpful
    1
  • Matthew Sheldon•320
    @MatthewNgabiranoSubmitted 16 days ago
    What are you most proud of, and what would you do differently next time?

    Finally, another project is done within two days. Next time am planning to add js on every part of the project I do.

    What challenges did you encounter, and how did you overcome them?

    I encountered the challenge of aligning displaying the image on top of the parent so that the rest of the children are below it but I finally got by looking into the code to see where the problem was.

    What specific areas of your project would you like help with?

    I am currently learning js and it would be the great part of the area I would like help with.

    Article-preview-component

    #accessibility
    1
    P
    schindlerdumagat•250
    @schindlerdumagatPosted 12 days ago

    Upon checking the live site, it seems that this project is not yet finished. I can still see some inconsistencies like the colors, spacings, responsiveness, etc. But I would like to give some feedback on some of the things that I can see right now as incorrect and provide some recommendations.

    Here are my recommendations:

    • It is better for performance to link fonts in HTML head tag instead of link it in the CSS file because it is the HTML file that the browser receives first before anything else.
    • You should put all the contents of your HTML body tag in a landmark role for accessibility reasons. In this particular, you should enclose the contents of the body element in a main tag. You can read more about landmark roles on the internet and see why it is necessary.
    • You should use heading tags in order (from h1 to h6). Don't use heading tags just to make the font size of a text smaller or bigger. You can do that in the CSS instead.
    • Even if you are already learning JavaScript, you should still revisit your HTML and CSS because it seems like you have missed some of the most important concepts and knowledge when working with HTML and CSS.
    Marked as helpful
    0
  • P
    Mars - Martiniano Leguizamon•255
    @martinianolSubmitted about 1 month ago
    What are you most proud of, and what would you do differently next time?

    I’m proud of the clean, responsive design and well-structured Sass. Next time, I’d refine accessibility and optimize animations for a smoother experience.

    What challenges did you encounter, and how did you overcome them?

    One challenge was aligning the button to the right in the footer. I fixed it by using margin-left: auto instead of justify-self, which only works in CSS Grid. Debugging layout issues with Flexbox helped me understand its behavior better. 🚀

    What specific areas of your project would you like help with?

    Images are not exactly as the prototype but I believe the images provided are not the correct ones...

    Responsive Landing Page | HTML, Sass, Flexbox & Media Queries

    #sass/scss
    1
    P
    schindlerdumagat•250
    @schindlerdumagatPosted about 1 month ago

    Here are my feedbacks:

    • I recommend that you use some CSS methodologies like BEM in styling your elements for better readability and code structure. But you can use other CSS methodologies as well if you want.
    • For the logo, you can use img tag with alt attribute value of "meet" for better accessibility.
    • I think using the article tag for the hero text content is not right since it doesn't stand out when this is moved to another page. And since it doesn't have an aria label, it is just like div. So, it is better to use div instead.
    • It is better to import fonts in the HTML head element instead because HTML is the first resource the browser reads. Thus, it is faster to load.
    • You can further breakdown you styles.scss by creating another scss partial file called _variables.scss or whatever you want to call it and just import it to your styles.scss for better readability and for separation of concerns. You can check the sass documentation on how to implement this. Just some nice tips.
    • Avoid using fixed units like px on your font sizes, use rem instead. Font sizes set in rems adapt automatically to user preferences. I recommend you read this resouce.
    • Avoid styling elements by their ids, use classes instead. Ids have a higher specificity than classes and it would be difficult to change styles later on.
    • I also recommend not to style elements by their tag name for maintainability reasons. For example, you want to change the element used in your html for accessibility reasons, you would have to change it in your CSS as well. You can use class instead for easier maintainability.
    • Its better to use unitless values on line heights as it scales properly according to the font size.
    • Don't use px in media queries, use rem or em instead.
    • I recommend that you read the benefits of using relative units instead of px for you to become wiser in what unit to use. I have seen in your CSS file that you use px in most of the time which is not a good idea.
    • There are also some modern CSS resets on the internet which can check or even add to your code. Just for you to be aware.
    • As for your concern on the images in the hero section, you can check on others code, understand it and then apply it to your code.
    Marked as helpful
    1
  • P
    schindlerdumagat•250
    @schindlerdumagatPosted 4 months ago

    I recommend that for each reviews in quotes, you can use a blockquote or q element instead of div with p element in it for better semantic meaning.

    Your usage of class to create the base styles for the card is great but I recommend you check out Sass's mixin feature which helps with CSS reusability. You might want to use that in the future.

    1
  • P
    Rinta Roy•100
    @rinta-gitSubmitted 4 months ago
    What challenges did you encounter, and how did you overcome them?

    Positioning the cards in the desktop view to match the design was quite challenging. The grid-row property of CSS Grid proved to be particularly useful in this situation.

    Feature card page using SCSS

    #sass/scss
    1
    P
    schindlerdumagat•250
    @schindlerdumagatPosted 4 months ago

    I think it is better to put the "Reliable effective ..." title and the paragraph below it in a header element and the cards in the main element. You can also improve the semantics of your code by setting each card as a section element. As much as possible, avoid using divs. If you can use semantic elements, use them.

    To further improve the view on tablet screen size, you can make a 2x2 grid to place the cards to utilize the remaining space and not leave it empty. You can do this even though it is not in the design.

    Marked as helpful
    1
  • theMaskedOtaku•10
    @FunkycodesSubmitted 5 months ago
    What are you most proud of, and what would you do differently next time?

    My biggest takeaway, I suppose, would be finally coming to the realization that there is a real world use case for the lessons evangelized in the layout bible; Every Layout

    What challenges did you encounter, and how did you overcome them?

    I kept forgetting to synchronize changes in my viewport values in html; picture tag media queries and whatnot, and those in css.

    Product preview card component

    #sass/scss#vite
    1
    P
    schindlerdumagat•250
    @schindlerdumagatPosted 5 months ago

    Upon inspection, you forgot to add cart icon in the button and also make the button text color pure white and not cream.

    Aside from my visuals, I also checked your HTML and I recommend that you should practice using semantic elements than using div tags. I think you should use the main tag in your HTML to enclose all the contents within your body element. You should use at least one landmark element to contain some elements in your HTML for better accessibility. In this case, you should use the main tag because using header, footer or any other landmark semantic elements is not needed because we don't have header and footer in the design.

    Additionally, I think it's better to use the h1 tag for the product name because it is the title of the product. The word "PERFUME" is just a tag to identify what kind of product it is.

    0
  • P
    schindlerdumagat•250
    @schindlerdumagatPosted 5 months ago

    Upon checking your html, I recommend that you should only h1 once. Put the "Simple Omelette Recipe" as the h1 and make "Preparation time", "Ingredients", "Instructions" and "Nutrition" as h2 because they are sub sections. It is a best practice to only use h1 once on every page and use h2 - h6 for the succeeding sub sections in order.

    Marked as helpful
    0
  • P
    schindlerdumagat•250
    @schindlerdumagatPosted 5 months ago

    I can see that there are still inconsistencies with the styles. I assume this is not yet done.

    Marked as helpful
    0
  • P
    schindlerdumagat•250
    @schindlerdumagatPosted 5 months ago

    I see that the positioning of the card is not in the center of the page but if you intend make it this way. That is ok.

    0
  • P
    schindlerdumagat•250
    @schindlerdumagatPosted 5 months ago

    The card is not positioned in the middle of the page. Upon inspecting your styles.css, you have used flexbox to move it to the center. There is just one thing missing, you haven't specified the total height of the body element which you specified as the flexible containing element by giving it a display of flex and it defaulted to the height of the card. Make sure to set the height of the body element to the total height of the browser window because flexbox also takes into consideration the height and width of the flexible containing element.

    You can do this by giving it a css rule of:

    height: 100vh;

    0