Skip to content
OverviewSolutions
3
Comments
16
Sanjeewa Marukona
@eewa-SANJ

All comments

  • Sanjeewa Marukona•200
    @eewa-SANJPosted almost 2 years ago

    Hello!. Your solution looks fantastic and looks exactly like the design. Here are fix issues I found

    1. You use mixed units such as %, em, rem, and px. Try to use rem with one other in order to maintain consistency.
    2. Order your CSS style properties in an alphabetical way. So, its readability will increase.

    That's it. I think, my findings will help you to improve your coding.

    Happy coding😊

    Marked as helpful
    1
  • Sanjeewa Marukona•200
    @eewa-SANJPosted almost 2 years ago

    Hello!. Your solution looks fantastic and I have gone through your code. There are a few issues found in your codes. Here are my findings.

    1. Your main large image and other text and button overlap each other. Try to fix it.
    2. In smaller screen sizes, h1, p, and buttons are not verticle centered. you can align it by adding align-item: center into the flex div container. (class = "row" in your file)
    3. Social icons are not styled properly. You have to add a border and some padding to isolate inside a circle.
    4. In "Register" you don't add the box shadow. Do see that?
    5. Learn to use rem instead of using px. It is a suggestion and best practice.

    I think, my findings will help you to improve your coding.

    Happy coding😊

    Marked as helpful
    2
  • Sanjeewa Marukona•200
    @eewa-SANJPosted almost 2 years ago

    Hello!. Your solution looks fantastic and I have gone through your code. There are a few issues found in your codes. Here are my findings.

    1. In the "Proceed to payment" button rendered the different font family. try to Fix it
    2. Music image is only for decorative purposes so, img alt tag can be blank in order to assist screen readers in skipping by adding area-hidden = "true".
    3. Also, In the "Proceed to payment" button, the hover color is different than in the original design files.

    I think, my findings will help you to improve your coding.

    Happy coding😊

    1
  • Sanjeewa Marukona•200
    @eewa-SANJPosted almost 2 years ago

    Hello!. Your solution looks good but I have gone through your code. There are a few issues found in your codes. Here are my findings.

    1. Your site is not responsive, so it crashes and overlap here and there. You can fix those issues using media queries. If you want to help me more, contact me and I will help you

    2. Your favicon is not rendered due to the wrong path. Could you fix it?

    3. Your main image(image web 3 desktop) only rendered in small screens. But on small screens, your mobile version of that image should be rendered. <img srcset="assets/images/image-web-3-mobile.jpg 686w, assets/images/image-web-3-desktop.jpg 1460w" sizes="(max-width: 375px) 686px 1460px" src="assets/images/image-web-3-desktop.jpg" alt="Web 3.0" > . If you want how to implement this Read this

    4. There is an alignment issue in your sidebar and below three column divs. You can adjust it by resizing the sidebar according to the three-column width.

    5. You have missed the mobile hamburger menu in the tab and mobile screen sizes

    I think, my findings will help you to improve your coding.

    Happy coding😊

    Marked as helpful
    1
  • Sanjeewa Marukona•200
    @eewa-SANJPosted almost 2 years ago

    Hello!. Your solution looks fantastic and I have gone through your code. There are a few issues found in your codes. Here are my findings.

    1. You don't set ALT attributes in some of your images. Try to set proper alt attributes in your coding because it is important for accessibility reasons as well as SEO
    2. You don't put the default CSS reset coding in a CSS file. It is essential to reset the default CSS in order to minimize conflicts with your custom CSS. [Read this
    3. In small screen sizes, your social media icons still right side so you fix that to the center to look more consistent
    4. Also, the logo is centered on small device sizes, which is different than the design files. You can ignore this only suggestion

    I think, my findings will help you to improve your coding.

    Happy coding😊

    0
  • Sanjeewa Marukona•200
    @eewa-SANJSubmitted almost 2 years ago

    I completed the News Home Page challenge. I have a little bit of confusion while coding the mobile hamburger menu.

    *Do you have another approach to coding the hamburger menu?

    I invite you to review my code and give me feedback. I appreciate it.

    Thank you

    News Home Page

    2
    Sanjeewa Marukona•200
    @eewa-SANJPosted almost 2 years ago

    Thank you for the suggestions, I have to fix the scrolling issue, and want to figure out how to fix it

    0
  • Sanjeewa Marukona•200
    @eewa-SANJPosted almost 2 years ago

    Hello! Your solution is good and looks nice but there are a few issues in your solution. Here are my findings.

    1. You are missing the background color on the "Change" link part. Apply the provided color for the background which is #F8F9FE
    2. In the "Proceed to payment" button, you don't apply the shadow you can apply the shadow using the box-shadow property
    3. Also, apply cursor: pointer for your links
    4. I suggest you use a rem unit other than px
    5. Also, I suggest you keep CSS code in an alphabetical way in order to improve readability

    These issues and suggestions might help you to improve your coding.

    Happy coding😊

    0
  • Sanjeewa Marukona•200
    @eewa-SANJPosted almost 2 years ago

    Hello! Your solution is pretty nice. Here are some of my issue findings and suggestions.

    1. You can't use CSS-reserved keywords inside the class name. Your used <div class="flex"> this one. The error will not give but its not good practice
    2. You use mix of rem and px units, I suggest you use one unit in order to maintain consistency
    3. I notice that small screens and background patterns are repeated through the x-axis, so it is not mentioned in the design files. Tyr to fix it using background-size property and background-repeat property

    Overall your solution is amazing and clean. I think above mention suggestions will help you

    Happy coding😊

    Marked as helpful
    1
  • Sanjeewa Marukona•200
    @eewa-SANJPosted almost 2 years ago

    Hello!

    Your solution is nice and clean but there are some issues. Here is my findings,

    1. There is the wrong font family loaded in the "Proceed to payment" button. So, you have to declare the font family separately for that element
    2. There is a drop shadow in the "Proceed to payment" button in the original design. But your solution not includes that. You can apply the box shadow using this "box-shadow" property
    3. You can't apply the outline property to "None" due to accessibility reasons. It is only a suggestion. Check this A11Y

    I think you will receive some help from the above explanation.

    Happy coding😊

    Marked as helpful
    0
  • Sanjeewa Marukona•200
    @eewa-SANJPosted almost 2 years ago

    Hello, Your solution is quality and I have one suggestion for you which is, to use a rem unit for your design instead of px

    Happy coding😊

    Marked as helpful
    0
  • Sanjeewa Marukona•200
    @eewa-SANJPosted almost 2 years ago

    Hi, I have found some issues regarding your solution. Here are my findings

    1. There is a shadow all around the "Register" button, which is a lot different from the design. It is not an issue but a suggestion to fix it to be suitable for the design

    2. In the social icon section, there is no space between each item, try to give some margin for each item suitable way

    3. Also, try lowering line height to give a decent looking for the main heading

    4. In mobile screens, there are a lot of spacing issues. For example, there is a lot of pace between the Logo and the main image.

    I think these suggestions, help you to improve your coding Happy coding😊

    0
  • Sanjeewa Marukona•200
    @eewa-SANJPosted almost 2 years ago

    Amazing work! Here is my suggestion to make some improvements

    1. Main heading color is different than the original design, try to fix it
    2. As you can see, there is an alignment issue in " Annual Plan $59.99/year" part
    3. When hovering over the "Proceed to payment", there is the border is still displayed in another color. Try to fix it by applying the same border for the hover effect or remove completely from the border from the button if don't mind.

    I think this will help to improve some issues Happy coding😊

    Marked as helpful
    0