Skip to content
OverviewSolutions
4
Comments
22
Shivraj K
@Shivraj-K09

All comments

  • Shivraj K•400
    @Shivraj-K09Posted over 1 year ago

    The HTML and CSS code look great!

    • The structure of the HTML is simple and straightforward, and the CSS styles are well-organized and commented which makes it easier to understand.
    • The media query for larger screens is well-implemented and effectively changes the layout to make it more visually pleasing on larger screens.

    Overall, great job on the code and Happy Coding 😉

    Marked as helpful
    0
  • Shivraj K•400
    @Shivraj-K09Posted over 1 year ago

    The HTML and CSS code looks well-structured and commented, which is great for readability and maintainability.

    • In terms of the responsive design, it seems like you are trying to use a media query to make the layout responsive for screens wider than 500px. However, the syntax for your media query is incorrect. Instead of @media (width > 500px), it should be @media (min-width: 500px). Also, remember to include the appropriate closing tag (}) to close the media query.
    • Additionally, it might be helpful to add some responsive styling to the .image class to adjust the height property and maintain aspect ratio for different screen sizes.

    Overall, great job on the code and Happy Coding 😉

    0
  • Shivraj K•400
    @Shivraj-K09Posted over 1 year ago

    Great job on the code! Here are some positive observations:

    • Your HTML is well structured, and you're using semantic HTML elements where appropriate. You're also linking to external resources (fonts and images) using proper URLs.
    • Your styles are well-organized and utilize readable class and selector names.
    • You're including media queries for responsive design at appropriate breakpoints.

    Here are some recommendations for improvement:

    • Consider adding some padding to your body element so your content doesn't go all the way to the edge of the viewport.
    • In your .container-main selector, consider using margin: 0 auto instead of align-self: center to horizontally center the container.

    Overall, great job! Keep up the good work! Happy Coding😉

    Marked as helpful
    1
  • Shivraj K•400
    @Shivraj-K09Posted over 1 year ago

    Overall, the code structure is good and easy to understand. You have used appropriate HTML tags and included a meta viewport tag for responsiveness. Your CSS selectors are also properly named.

    However, there are some areas of improvement that I would like to mention:

    • In the style.css file, the font size unit for .how selector is missing (it currently reads as font-size: px;), which may cause issues in your webpage's display. You should edit this line to include a valid font-size unit such as px, em, or %.
    • The max-width property for .container selector has a fixed value of 360px, which might be too small for certain devices. Instead, you can use a max-width value in em or % units to make your webpage more responsive.
    • The justify-content property is not applicable to the .attribution selector as it is currently a div element. You may want to either give it a display of flex or use text-align: center instead.
    • In the attribution section, the challenge link to "Frontend Mentor" should have the rel="noopener noreferrer" attribute added to the <a> tag. This attribute is used as a security measure to protect against vulnerability exploits related to link hijacking, by preventing the new page from being opened in the same window, potentially allowing the destination site to take control of the source site.

    Other than these minor issues, your code looks good and functions as intended. Good job and Happy Coding 🫡😉

    0
  • Shivraj K•400
    @Shivraj-K09Posted over 1 year ago

    Based on the code, here is my feedback:

    • Great use of semantic HTML. You have used appropriate tags such as nav, ul, and picture which makes your code easier to read and understand.
    • Your CSS code is structured well and easy to read. You have used appropriate class names and have grouped related styles together.
    • Use of media queries for responsiveness is great. However, in some cases, you are relying on fixed values such as min-width: 600px which might not work well across different devices. It's a good idea to use relative values such as em or % to ensure consistency across devices.
    • Consider reducing the number of font sizes used in your code. Having too many font sizes can make your code harder to read and maintain. Try to group related font styles together and use CSS selectors to apply them to different elements.

    Overall, great job on your code and keep up the good work! Happy Coding 😉🫡

    Marked as helpful
    1
  • Sanjeet Rai•100
    @Sanjeet204Submitted over 1 year ago

    I was quite addictive of using position to relocate elements to their perfect location however, this time taken a challenge to use less position and manage to perform to task beautifully using margin and padding instead of position.

    In terms of interactivity it was quite easy only needed to validate email. Please share your views regarding this challenge .

    Mail validation using event listener

    3
    Shivraj K•400
    @Shivraj-K09Posted over 1 year ago
    • Great job! It's good to see that you were able to achieve the desired layout using margin and padding instead of relying solely on positioning.
    • One suggestion I would make is to use const instead of let when declaring pattern since the variable doesn't need to be reassigned. It's a small change, but it can help communicate to future readers of the code that the variable is intended to be constant.

    Keep up the good work! and Happy Coding 😉🫡

    Marked as helpful
    0
  • Shivraj K•400
    @Shivraj-K09Posted over 1 year ago

    Here are some suggestions to further improve the code:

    • Accessibility: Add a descriptive alt attribute to the img element. This will help screen readers give more context about the image to users with accessibility needs.
    • Use semantic HTML: Consider using more semantic HTML tags. Instead of the card tag used on the wrapper, use a standard HTML tag like div. This will make your code more accessible and easier to understand for people using assistive technologies like screen readers.
    • Organize your CSS file: Consider grouping your CSS rules together based on their purpose, for example, group all the styles for the .top and .bottom class into two separate blocks. This makes it easier for someone reading the CSS file to understand which styles are being applied to which part of the HTML. Happy Coding 😉
    Marked as helpful
    1
  • Shivraj K•400
    @Shivraj-K09Posted over 1 year ago

    The code looks good and well structured. Here are some suggestions to further improve the code:

    • Accessibility: Add a descriptive alt attribute to the img element. This will help screen readers give more context about the image to users with accessibility needs.
    • Use semantic HTML: Consider using semantic HTML tags like <header>, <main>, and <section> instead of <div>s. This will make your code more accessible and easier to understand for people using assistive technologies like screen readers.
    • Consistency: For consistency purposes, try to use either a hyphen or an underscore when naming classes. In this code snippet, both conventions are used, which can be confusing for future developers working on the code. And Happy Coding 😉
    Marked as helpful
    1
  • Shivraj K•400
    @Shivraj-K09Posted over 1 year ago

    Your HTML and CSS code look correct, and the product preview card component looks good. However, here are some suggestions that you could consider -

    • Use semantic HTML elements like header, main, nav and footer to improve the readability and accessibility of your code. You might want to consider reducing the font size of your content on smaller screens. A font size of 14px on smaller screens may result in poor legibility.
    • You may also want to consider adding a media query to adjust the dimensions and layout of the section element based on the device screen size to enhance user experience.
    • Otherwise, your code looks good and is well-organized. Happy Coding 😉
    Marked as helpful
    0
  • Wessel Konstantinov•350
    @WesselKonstantinovSubmitted over 1 year ago

    This is my solution to the QR code component challenge. I decided to include a CSS reset to set up some default styles. I looked into Andy Bell's one for this. This reset contains styles referring to tags that I did not use in this challenge. For example:

    /* Inherit fonts for inputs and buttons */
input,
button,
textarea,
select {
  font: inherit;
}

    Should I leave out the default styles that are not relevant to the styling of the QR code component or is it better to always include the entire CSS reset?

    I am looking forward to your feedback and suggestions!

    QR code component using CSS custom properties and flex box

    2
    Shivraj K•400
    @Shivraj-K09Posted over 1 year ago
    • It is not necessary to include the entire CSS reset if you are only using a small portion of it. In your case, you could remove the styles that are not relevant to your QR code component.
    • However, keeping the entire CSS reset may be beneficial in case you decide to use those styles elsewhere in your project. It is a matter of personal preference and project requirements.
    • It's great to see that you are paying attention to CSS best practices and leveraging a CSS reset to ensure consistent styles across different browsers. Keep up the good work! and Happy Coding 😉
    Marked as helpful
    2
  • Jason Ritter•40
    @HigokianSubmitted over 1 year ago

    Questions

    1. Nesting

    Any tips on how to determine how many layers will be needed? Specifically when for flex-box/boostrap? This is probably the biggest thing I struggle with.

    2. Centering Card

    This question kind of ties in the the nesting question. My biggest issue was getting the card to center horizontally and vertically. I ended up giving the body class="d-flex flex-column justify-content-center" and then a container nested within that with class="d-flex justify-content-center". Is there a better solution to this?

    I appreciate any and all feedback!

    Responsive QR Code Component Using Bootstrap

    3
    Shivraj K•400
    @Shivraj-K09Posted over 1 year ago
    • Nesting: The number of layers needed for your HTML structure will depend on the complexity of your layout and the design requirements. As a general rule, you should try to keep your HTML structure as simple as possible, without sacrificing the design. For example, in the code you provided, you have used Bootstrap classes to style your card container, which makes it easier to manage the layout.
    • Centering Card: Using Bootstrap classes like d-flex and justify-content-center is a good way to center your card horizontally and vertically. However, if you need to center an element in the middle of the page without using Bootstrap, you can give the parent container a fixed height and use flexbox to center the child element. For example:
    body {
  display: flex;
  justify-content: center;
  align-items: center;
}

.container {
  height: 100vh;
  display: flex;
  justify-content: center;
  align-items: center;
}
    • This will center the .container element both horizontally and vertically on the page.
    • Overall, I think your code looks great and you are using Bootstrap classes effectively. Keep up the good work! Happy Coding 😉.
    Marked as helpful
    1
  • Shivraj K•400
    @Shivraj-K09Posted over 1 year ago

    Your HTML and CSS code look great! Here are a few suggestions on how you can improve further:

    • Use more semantic HTML elements: Instead of using generic div elements, you can use more semantic elements like header, section, article, and footer. This makes your code more readable and accessible.
    • Use responsive images: Adding responsive images with the srcset and sizes attributes can help optimize your website's loading speed and improve user experience.
    • Write mobile-first CSS: Writing CSS with a mobile-first approach means that you focus on styling for smaller screens first and then add on styles for larger screens. This helps you avoid styling conflicts and makes your website more responsive.
    • Add more accessibility features: Ensure that your website meets accessibility standards by adding alt text for images, using semantic HTML, and testing keyboard accessibility.
    • Use CSS variables: CSS variables, or custom properties, can make your CSS code more modular and easier to maintain. You can define a variable for a color, font size, or any other property, and then use it throughout your code.
    • Overall, your code looks great and you have followed best practices in HTML and CSS. Keep up the good work!
    Marked as helpful
    0