Skip to content
OverviewSolutions
5
Comments
7
Jakub Jirous
@jakubjirous

All comments

  • Andrew Hein•80
    @Heinz157Submitted almost 2 years ago

    I found the css super difficult to handle. Still not perfectly where I want it but the functionality and looks are there.

    Desktop first Calc app

    2
    Jakub Jirous•270
    @jakubjirousPosted almost 2 years ago

    Hi Andrew,

    I have reviewed your code for this cool calculator app and here is my constructive feedback:

    HTML:

    • You have a typo in the class name coantainer. It should be container.
    • You are using an obsolete way of defining the form name attribute. Instead of name, you should use id and access the form using document.getElementById("formId").

    CSS:

    • Consider using more meaningful class names to make it easier to understand what each element represents.

    JavaScript:

    • You can use event delegation to avoid having to add event listeners to each button individually.
    • Instead of concatenating the input value, you can use the button value property to directly access the button value and append it to the input.
    • At the end, there is a little bug, when you click equal button directly, I am getting undefined on the calc screen

    Keep up the great work and happy coding!

    Marked as helpful
    1
  • Jakub Jirous•270
    @jakubjirousPosted almost 2 years ago

    Hi Raul,

    Overall, the HTML and CSS code you provided for the Results Summary component looks good! Here are some constructive feedback and suggestions to help improve it:

    Add alt attributes to images: It's a good practice to add descriptive alt attributes to images for accessibility purposes. Consider adding alt attributes to the images in the Summary section.

    Avoid using IDs for styling: While IDs can be useful for targeting specific elements with JavaScript, they should be avoided for styling purposes. Instead, use classes or element selectors.

    Use consistent naming conventions: In some places, you use hyphens to separate words in class names, while in others, you use underscores. It's a good idea to use consistent naming conventions throughout your code.

    Keep up the great work and happy coding!

    Marked as helpful
    0
  • Karen Lourenço•380
    @KarenMascarenhasLourencoSubmitted almost 2 years ago

    Is my code understandable?

    Did I use the semantic HTML tags correctly, should I add more or less?

    In what areas of my code can I improve on?

    All feedback is greatly appreciate. It helps me to improve as a frontend developer. Thanks!

    Social Proof Section

    1
    Jakub Jirous•270
    @jakubjirousPosted almost 2 years ago

    Hi Karen,

    First of all, the HTML code provided is well-structured and organized. The use of semantic tags such as main, section, h1, h2, p, and footer is appropriate and helps in understanding the layout of the page.

    The CSS code is also well-organized and follows best practices by using * { margin: 0; padding: 0; } to reset the default margins and padding of all elements. It's good to see that the author has used descriptive class and ID names to style the different elements of the page.

    However, there are a few areas where the CSS code could be improved. Firstly, the background-image property is set twice in the body selector. It's unnecessary to set the background image twice, and the second declaration will overwrite the first one.

    Secondly, the grid-template property of the main selector is not complete. It's recommended to specify the number of rows and columns that the grid should have. For example, grid-template-columns: 1fr 1fr; would create a grid with two equal columns.

    Overall, the provided HTML and CSS code is of good quality and follows best practices. With a few minor improvements, it could be even better.

    Keep up the great work and happy coding!

    Marked as helpful
    0
  • Jakub Jirous•270
    @jakubjirousPosted almost 2 years ago

    Hi Jasenko,

    Overall, the code provided for the Interactive Rating Component looks well-structured and easy to read. The HTML and CSS follow best practices, such as using semantic HTML elements and using CSS variables to define colors. However, there are a few potential areas for improvement that could be addressed to further enhance the code.

    1) Accessibility – The rating buttons are currently represented using button elements, which is good practice. However, there are no visible labels associated with each button to describe their purpose to screen readers. Consider using aria-label attributes to provide accessibility for users who rely on assistive technology.

    2) Inline Styles – In the HTML code, the span element with ID rating has inline styles applied to it. It is generally considered better practice to use a separate CSS file and apply styles there rather than inline styles.

    3) CSS Import – The CSS file is being imported using an @import rule in the CSS code. This can slow down page load times as it requires an extra HTTP request. Consider using the <link> tag in the HTML file instead to load the CSS file.

    4) Focus styles – The code uses a focus style for the submit button but not for the rating buttons. Including a focus style for the rating buttons can improve the user experience for keyboard users.

    You did an excellent job in addition to that!

    Cheers, Jakub

    Marked as helpful
    0
  • Jakub Jirous•270
    @jakubjirousPosted almost 2 years ago

    Hi Jackson,

    Overall, the solution in style.css looks well-written and adheres to some best practices, such as the use of CSS variables and the box-sizing property.

    Here are a few specific comments that could improve the code:

    1) Consider consolidating your font weights: The Overpass font family includes 20 different weights, but you may only need a few of them. Depending on your use case, you might be able to simplify the @import statement and reduce the size of your CSS file.

    2) Use more descriptive class names: The current class names, such as card-one and card-two, are not very informative. Consider using more specific names, such as quiz-card and result-card, to make it clear what each section of the page represents. Consider reordering your properties for better readability. For example, you might group all of the font-related properties together and all of the background-related properties together to make it easier to scan the code.

    3) Use more semantic HTML: The current HTML code doesn't use many semantic elements. Consider restructuring the code to use these elements and make it easier to understand the structure of the page.

    However, other than that, you did an excellent job!

    Cheers, Jakub

    0
  • Jakub Jirous•270
    @jakubjirousPosted almost 2 years ago

    Hi Tales,

    Overall, your code looks well-organized and readable. Here are a few suggestions for improvement:

    1) Comment your code: It's always a good idea to add comments to your code, especially if you're working in a team. Comments help others understand your code and make it easier to maintain. Consider adding comments to explain the purpose of each section of your code.

    2) Use descriptive names for classes and variables: Your class and variable names should describe what they represent. For example, instead of using the class name "container", you could use "card-container" to be more descriptive.

    3) Group related properties together: When you're defining styles for an element, group related properties together to make it easier to read and maintain. For example, you could group all of the styles for the "ratings" section together, rather than spreading them out.

    4) Use consistent formatting: Consistent formatting makes your code easier to read and understand. Make sure you're using the same formatting throughout your code, including indentation and spacing.

    However, other than that, you did an excellent job!

    Cheers, Jakub

    Marked as helpful
    0
  • Jakub Jirous•270
    @jakubjirousPosted almost 2 years ago

    Hi, It's obvious that you have a good understanding of the problem and have worked hard to find a solution. I greatly appreciate the effort you have put into providing this solution; your code is clear and easy to understand, and your logic is sound.

    To make your code even more readable and maintainable, I would recommend using a tool that facilitates code formatting. Despite the fact that your current formatting is adequate, using a code formatter will help others understand and read your code more easily, especially if you are part of a team.

    Using tools such as Prettier, you can automatically format your code according to a set of rules, ensuring consistency and readability across your project.

    Overall, great job on the solution, and I encourage you to consider using a code formatter to make your code even better!

    Cheers, Jakub

    Marked as helpful
    0