Skip to content
OverviewSolutions
8
Comments
3
ChrisEski
@ChrisEski

All comments

  • ChrisEski•110
    @ChrisEskiPosted almost 3 years ago

    Hey @do-Va!

    Really nice solution over there, congratulations!

    I liked the smooth transition to the accent color on hovering over the title and the name, but i would add this also when hovering over the image. This would make your design more consistent! And last it would be better if you added cursor: pointer on all 3 hovered elements!

    Well done :)

    1
  • ChrisEski•110
    @ChrisEskiPosted almost 3 years ago

    Hello saira, first of all good job over there! I'm goint to list my remarks below:

    1. As for the HTML part, i think it's a better practice to use classes instead of ids. By using ids i believe you might mess with specificity, especially in bigger projects.

    2. About the card div, setting the height in 'vh' units makes it to change the card's height as the screen size changes, i.e. for small/normal size smartphones it's probably goint to be ok, but as the screen gets taller and bigger, such as a desktop, you notice that the card height increases leaving the content sticked in their initial position. You want the card's height to stay the same. Maybe you want the width to change with the viewport, but in this project the change is not that big. Plus there is some content overflow as the card height gets smaller than the content height

    3. In my opinion, this card seems to be like a preview card of an article or something, which makes me believe that it would be more relevant to make the image and the title as links, that lead to that specific article or a further info section. Saying this, the cursor should be a pointer on hover to make it clearer that these elements have some functionality. Same with the creator's name, which should probably be a link that takes you to his/her profile or show info about him/her.

    4. Now this is a personal preference, but i think that it makes for a better experience if you give the hover effect a smoother transition.

    5. Last, since you are working on scss (which i am starting to love), consider dividing your style code in partials and use some more of the nesting thing, cause this is where sass really shines! It will make your code cleaner, more organized and easier to refactor/debug in the future.

    These were my humble notes on your project. Hope i helped just a liitle bit there... :)

    Marked as helpful
    0
  • ChrisEski•110
    @ChrisEskiPosted almost 3 years ago

    Hey there, the overall outcome is really nice, congrats on that!

    Now about what i have noticed:

    1. Since it's a card with some functionality on hover over the image, the title and the creator's name, i guess that these elements would be there to link you to a more detailed article or some kind of futher details about the crypto. So it should be better if you used them as <a> elements rather than <divs>

    2. This comes from the above, you should try using semantic html, which I'm not really good at, but the main idea is that you should have an <h1>, a <main> section, and generally use <div>s when there is not something more specific to use.

    3. About the accessibility I have not much to say since I also struggle there and I currently try to pay some attention on that too.

    4. What I've also noticed is that in your design the card size remains the same throughout the screens variations. As i can see from the design images, I'm pretty sure it ges only a little bit smaller on mobile as well as the typography changes, but not that much to be a problem imo.

    5. Last, which is not of any big importance, you should take some time to fix the links in the attribution section, so that anyone can reach you :)

    Nice work and keep on learning...

    0