Skip to content
OverviewSolutions
28
Comments
37
Manish Patel
@savvystrider

All comments

  • Manish Patel•740
    @savvystrider

    If you're using SVG code in your HTML, you can change the fill or stroke color directly in the markup or through CSS. If you're importing the SVG as a file, you can open up the code in any text editor and change the stroke and/or fill colors

    Marked as helpful
  • Manish Patel•740
    @savvystrider

    The page looks great and the responsiveness functions well. The only thing I noticed is that the font is not displaying. Looks like the import is commented out in your index.css file:

    /* @import url('https://fonts.google.com/specimen/Epilogue'); */ @font-face { font-family: "Epilogue"; src: local ("Epilogue") url(../fonts/Epilogue/Epilogue-Italic-VariableFont_wght.ttf) format("truetype"); }

  • Manish Patel•740
    @savvystrider

    The easiest way I've learned to properly center an element like this is to apply the following styles to the body element: display: grid; min-height: 100vh; place-items: center;

    Marked as helpful
  • Manish Patel•740
    @savvystrider

    Looks great! The only thing I would recommend is adding some transitions to the hover effects/active states.

  • Manish Patel•740
    @savvystrider

    Wow, looks amazing! I noticed a few minor issues with the modal:

    • clicking anywhere inside the modal closes the modal
    • scrolling up and down with the modal open causes the background to scroll up/down
  • Manish Patel•740
    @savvystrider

    Your project looks great! To get your background images to appear, you will have to make a minor modification to your CSS:

    .circle-background and .circle-background2 need a period added inside the parentheses for the url

    background-image: url('./images/bg-pattern-bottom.svg') instead of background-image: url('/images/bg-pattern-bottom.svg').

  • Manish Patel•740
    @savvystrider

    Looks awesome! The main text is a bit hard to read against the background (not enough contrast), so I'd recommend using a darker color to make it stand out.

  • Manish Patel•740
    @savvystrider

    Your design looks good but the images are not displaying. Change the main image to <img src="image-product-desktop.jpg" alt="perfume"> and the icon to <img src="icon-cart.svg" alt="cart"> to get them to display properly.

  • Manish Patel•740
    @savvystrider

    Your solution looks awesome and the form validation works really well. The only thing I'd suggest is preventing users from typing letters into input fields intended for numbers. You can achieve that with HTML.

  • Manish Patel•740
    @savvystrider

    I'd recommend using the object-fit CSS property: https://developer.mozilla.org/en-US/docs/Web/CSS/object-fit to handle images.

  • Manish Patel•740
    @savvystrider

    The text content looks good but it looks like you did not upload an image with your code.

    Marked as helpful
  • Manish Patel•740
    @savvystrider

    Looks awesome! If you're having trouble with mobile layouts, you can try simulating different screen sizes with your web browser. Google Chrome does it pretty well: https://developer.chrome.com/docs/devtools/device-mode/.

  • Manish Patel•740
    @savvystrider

    The file path of your img is incorrect, so it's not showing up. Change the src attribute to src="./qr-code-component-main/images/image-qr-code.png".

  • Manish Patel•740
    @savvystrider

    The design came out well! I'd recommend the following in your HTML:

    • Add alt text to all images for accessibility.
    • Use relative paths for file references. This will ensure you don't have issues with file paths if you change hosts.
    • Use more semantic tags, like header, nav, footer, or section for accessibility.
  • Manish Patel•740
    @savvystrider

    Looks good! I'd recommend not using pixel units for width in your media queries. Consider using relative units of measurements, like em or rem or percentags. Seems like you are using them in other parts of your code already.

    Marked as helpful
  • Manish Patel•740
    @savvystrider

    Looks excellent! Small suggestion:

    Avoid using floats for layout: Instead of floating the icon to the right in .main-card__icon, consider using flexbox or grid to position it.

  • Manish Patel•740
    @savvystrider

    Looks amazing!

    Here are some suggestions to improve the JS:

    • Use const instead of let for variables that are not reassigned.

    • Use textContent instead of innerHTML since you're only setting text content.

    • Use input event instead of click event for input fields: Instead of attaching the click event to increase or decrease the number of visitors, you can attach the input event to the visitorNumber input field. This allows users to change the value by typing directly into the input field.

    • Instead of showDiv1(), showDiv2(), and showDiv3(), you could use more descriptive function names in camelCase, such as showFamilyBooking(), showSpecialBooking(), and showSocialBooking().

    • Also, try to consolidate repeated code: The code that checks if a value is valid and shows or hides the label could be consolidated into a separate function that can be reused for each input field.

  • Manish Patel•740
    @savvystrider
    • You can use background-size: cover to display the image as a background image and then use the cover property to scale it.
    • You can add max-width: 100% to all images to make sure that images don't get any larger than they are supposed to be.
    • You can also use the transform: scale property to scale the image.
    • Also, you can use the HTML picture element "to offer alternative versions of an image for different display/device scenarios."

    Hope this helps! I think your project came out well, regardless.