Skip to content
OverviewSolutions
26
Comments
4
yas
@yosrajalali

All comments

  • yas•390
    @yosrajalaliPosted over 1 year ago

    Hi Vivek🙋‍♀️ congratulations on successfully completing this challenge🎉.

    Regarding your question: when they say Mobile: 375px it doesn't mean that you have to have a media query for width of 375px, it just means that the width of screenshot that they provide us is 375px.

    any way if you want to optimize your page for mobile devices you should use media query, like this👇

    {
@media only screen and (max-width: 600px){
**put your codes that are related to the mobile design here**}
}

    this👆 block of code executes only If the browser window is 600px or smaller.

    you can change 600px to any other width, and also you can add more than one media query base on your design.

    I hope you find this helpful..

    0
  • yas•390
    @yosrajalaliPosted over 1 year ago

    Hi Ismail👋.You did great on this project, I think you have forgotten to add hover effect on the image.to do this, you need to add this👇 to your code..

    {
<div class="img__container">
<img
class="diamond__img"
src="images/image-equilibrium.jpg"
alt="diamond"
/>
<div class="middle">
<img class="icon" src="images/icon-view.svg" alt="view icon" />
</div>
</div>
}

    and this is for your Css👇

    {
.img__container:hover .middle {
opacity: 1;
cursor: pointer;
}

.middle {
transition: 0.5s ease;
opacity: 0;
position: absolute;
top: 50%;
left: 50%;
transform: translate(-50%, -50%);
-ms-transform: translate(-50%, -50%);
display: flex;
align-items: center;
justify-content: center;
background-color: hsla(178, 100%, 50%, 0.3);
border-radius: 0.5rem;
}

}
    • also you need to add width and height to the middle class that is exactly the same as the image size.

    I hope you find it useful..

    Marked as helpful
    0
  • yas•390
    @yosrajalaliPosted over 1 year ago

    Hi Bassem 👋. Congratulations on successfully completing the challenge!🎉

    just a couple of suggestions

    HTML

    • for your whole container it's better to use <main> tag instead of a <div>.main tag is an HTML5 tag which is supposed to represent the main content of website. it's better for SEO purposes. you can check this 👉MDN

    CSS

    • if you want to center the whole content of your page horizontally and vertically using display flex for body is the best choice, like the code below..
    body {
min-height: 100vh;
display: flex;
align-items: center;
justify-content: center;
}
    • (*) is a universal selector and it is used to give universal styles to all elements. if you use font-weight and background-color in this universal selector it means that it gets apply to all elements of the page. try to do this..
    body {
font-weight: 300;
background-color: rgb(227, 224, 224);
}

    Anyway your solution is great🙂

    I hope you find it useful😊

    Marked as helpful
    1
  • yas•390
    @yosrajalaliPosted over 1 year ago

    Hi 👋 Congratulations on completing the challenge!

    just a couple of suggestions:

    • for your whole container it's better to use <main> tag instead of a <div>.main tag is an HTML5 tag which is supposed to represent the main content of website. it's better for SEO purposes. you can check this 👉**MDN**

     

    • also using an article tag is a better choice instead of using a <div> for your boxes. we use a div If the content within the element doesn't have any special meaning or when we need just a container, it's not wrong or bad to use a <div>, It's just better to use tags that fit the actual content of your website.

     

    • when we have a two dimensional layout using grid system will make it easier to design the web page without having to use flexbox (using flexbox in this case makes it hard to work with). check this if you want to know more aboutCss grid 👉MDN

    I hope you find it useful 🙂

    Above all your solution is great.

    0