Skip to content
OverviewSolutions
6
Comments
6
Abhishek Gulab Bhikule
@Bhikule19

All comments

  • 4jdin•50
    @4jdinSubmitted almost 2 years ago

    This is my second website I've built and I really put a lot of effort into it.

    As with all my work, I would be very grateful if you would leave me some criticism or advice, because that way I can progress more easily. If you have any questions for me, feel free to ask.

    Great solution for beginners

    2
    Abhishek Gulab Bhikule•200
    @Bhikule19Posted almost 2 years ago

    Hey, there amazing work,

    • I found out that the website has space more than the viewer page, due to which you can see a scroll bar.
    • Now to get rid of the extra space and bring the card to the middle and avoid a scroll bar, I rewrote your code for the body element.
    • So this is your code:-
    body {
    padding-top: 250px;
    font-style: none;
    background-size: cover;
    background-position: center;
    background-color: #f3eae3;
    display: flex;
    justify-content: center;
    align-items: center;
}
    • And this is what I wrote for you:-
    body {
    min-height: 100vh;
    font-style: none;
    background-size: cover;
    background-position: center;
    background-color: #f3eae3;
    display: flex;
    justify-content: center;
    align-items: center;
}
    • Notice the difference? Not much right I just canceled out the padding-top property which was making the extra space and letting the unnecessary scroll bar and actually added one property i.e min-height: 100vh;
    • With this, the flex properties will make more sense and the card will be placed at the center of your web page. Give it a try

    If you found this helpful you can mark this cmt as helpful

    Marked as helpful
    0
  • Abhishek Gulab Bhikule•200
    @Bhikule19Posted almost 2 years ago

    Amazing work there, just to let you know the picture tag syntax can be more meaningful as it is used to manipulate img for different screen sizes.

    You can take a look at this how you can write picture tag the better way:-

    <picture>
        <source
          media="(max-width: 450px)"
          srcset="./images/pattern-divider-mobile.svg"
        />
        <img
          src="./images/pattern-divider-desktop.svg"
          alt="divider pattern svg"
          class="divider"
        />
      </picture>

    I hope you found it useful

    Marked as helpful
    0
  • Dana Kalčíková•90
    @danyczechSubmitted almost 2 years ago

    Hi there!

    My second task. Any suggestions? I was thinking about putting those images as the background-image, but finally, I let them as part of the HTML and used display: none when needed. I am not sure which solution is better or the best practice.

    Responsive component - product card - using flexbox

    2
    Abhishek Gulab Bhikule•200
    @Bhikule19Posted almost 2 years ago

    You can do this alternative method:- So what happening here is, if the screen width is more than 800px it will use the desktop version of the image that you will provide, whereas when the screen comes to less than 780px it will implement the mobile version of that image.

    <picture>
  <source media="(max-width: 780px)" srcset="mobile-image.jpg" />
  <source media="(min-width: 800px)" srcset="desktop-image.jpg" />
  <img src="desktop-image.jpg" alt="" />
</picture>

    You can visit:- Developer docs by mozilla

    If you found this insightful, you can mark this comment as helpful Thank you.

    Marked as helpful
    1
  • Abhishek Gulab Bhikule•200
    @Bhikule19Posted almost 2 years ago

    Hey there mate, I really liked the way you used the article tag to implement the title, paragraph, and CTA. An alternate for this will be mainly flexbox. By having two div inside the main element, you can provide them with the class left and right respectively, and by giving the [ display: flex ] property to the main container in which these two reside, you can easily manipulate them. If you found this helpful you can make this comment marked as helpful.

    Thank you.

    Marked as helpful
    0
  • 👾 Ekaterine Mitagvaria 👾•7,600
    @catherineisonlineSubmitted over 2 years ago

    Hello, Frontend Mentor community! This is my solution to the Base Apparel coming soon page.

    I appreciate all the feedback you left that helped me to improve this project. Due to the fact that I published this project very long ago, I am no longer updating it and changing its status to Public Archive on my Github.

    You are free to download or use the code for reference in your projects, but I no longer update it or accept any feedback.

    Thank you

    Base Apparel coming soon page

    8
    Abhishek Gulab Bhikule•200
    @Bhikule19Posted almost 2 years ago

    Hey Catherine, The work is amazing, just so you know the placeholder text did not inherit the font family. So to do the trick, you can do the following:- "input::placeholder { font-family: }" This way you give the font family style to the placeholder text too.

    0
  • Abhishek Gulab Bhikule•200
    @Bhikule19Posted almost 2 years ago

    Hey there, Congratulations on your work. I just looked at your code and got some things to suggest for you as a fresher developer I myself made such mistakes.

    1. Wrap the "Gabrielle Essence Eau De Parfum" inside the <h1> tag as it will show the browser that this is the page's title. p.s:- Never leave a page without a title.
    2. <p class="normal-price">$169.99</p> Don't wrap inline items with "p" tag let it be used only for paragraph stuff. Instead use <span> tag like this:- <span> <del> $169.99 </del></span> Also, the <del> tag defines the text that has been deleted from a document. Browsers will usually strike a line through the deleted text.

    Check out this below link for more understanding:- https://www.w3schools.com/tags/tag_del.asp#:~:text=The%20tag%20defines%20text,a%20line%20through%20deleted%20text.

    If you find this insightful in any way you can give it marked as helpful.

    Marked as helpful
    0