Skip to content
OverviewSolutions
7
Comments
6
Abdul
@abdulrahman-rwim

All comments

  • Anthony•355
    @ACdev27Submitted almost 3 years ago

    Any feedback is appreciated, especially any feedback related to use of flex box, or improving accessibility.

    Meet Landing Page

    1
    Abdul•255
    @abdulrahman-rwimPosted almost 3 years ago

    hey @ACdev27 good job solving this challenge, I've some feedback for this solution

    • the .photo-set__images are too big and they are causing overflow issue to avoid this problem give them a max-width .photo-set__image { max-width: 45%; }
    • and for the .introduction__images reduce the margin-left to maybe 6px to make them align perfectly Example: .introduction__image-1--tablet { margin-left:6px; }
    • I suggest adding transition: background 250ms; to the buttons, this will make: hover smoother.
    • I hope this was helpful for you, other than that, nicely done, happy coding 😄
    Marked as helpful
    0
  • Carl Jones•200
    @jones9411Submitted almost 3 years ago

    Had a little trouble positioning the two outer cards, tried using flex-box but it wouldn't change the position so ended up using margin-top. any idea on how I could have gotten it to work with flex-box?

    Four card landing page

    2
    Abdul•255
    @abdulrahman-rwimPosted almost 3 years ago

    Hey @jones9411, you can solve this problem using grid I'm not sure how to solve it with flex-box,

    • this will solve the problem without using margin .column.c { display: grid; align-content: center; } I hope this was helpful for you 😄
    Marked as helpful
    1
  • Abdul•255
    @abdulrahman-rwimPosted almost 3 years ago

    Hi @Yahir-am good job solving this challenge, I've got some feedback for this solution.

    • the background image does not cover the entire page to solve it try this line of code background: var(--violet) url("./images/bg-desktop.svg") no-repeat top left / cover;
    • check out this amazing article on the background property
    • avoid using height especially on images because that makes them stretch
    • wrap the illustration image with div and give it max-width:100% to make sure not to get bigger
    • give your text content a limit width max-width:40% to make sure not to get bigger
    • avoid using width and height on buttons use padding instead ,try this button { display: block; margin-top: 1.5em; padding: 0.7rem 5rem; }
    • I suggest adding transition: color 250ms; to the button and the links, this will make: hover smoother.
    • start with the mobile design first because that makes styling elements easier
    • lastly to fix the accessibility issue remove the script tag from the body and for the icons use this instead put a CDN link inside the head tag you can find it here font awesome CDN link
    • I hope this was helpful for you, other than that, nicely done, happy coding 😄
    Marked as helpful
    0
  • Abdul•255
    @abdulrahman-rwimPosted about 3 years ago

    Hi Kasia your solution for this challenge looks really awesome well done everything looks almost perfect, just one thing I noticed on the mobile view is the illustration image causing an overflow issue, to solve the problem: you can give the image a max-width:100%; to make sure not getting bigger, try this code: img.header__main-illustration { max-width: 100%; }, the font-size for .button--pink on the mobile view really small if you make it 16px will be perfect, and that's all, happy coding 😄

    Marked as helpful
    0
  • Amanda Toop•40
    @atoopdevSubmitted about 3 years ago

    Some of my text does not fully line up to match the design. Also, I did not make use of the Lexend Deca font - this felt wrong to me, but I couldn't seem to find a spot where it belonged.

    Stats Preview Card Using Flexbox

    1
    Abdul•255
    @abdulrahman-rwimPosted about 3 years ago

    Hi @atoopdev Great solution well done!

    • there are some notes to make it even better

    • there is an overflow on the page, the scroll bar on the bottom use: body { overflow-x:hidden; } to remove it

    • clear the accessibility issues try the role attribute on HTML element to describe the purpose of them example: <div class="content-wrapper" role="textbox">,

    • and I recommend starting with the mobile design first

    0
  • Abdul•255
    @abdulrahman-rwimPosted about 3 years ago

    hi @peter-kalavritinos good work on this challenge your solution looks great on desktop, but it's not responsive, take a look again on the mobile view, and to get rid of accessibility issues,

    • here are some solutions
    • use semantic HTML tags <main class="main"> instead of <div class="main"> <footer class="attribution"></footer>
    • correct the HTML validation ont-size: 112x; you missed a Letter here should be font-size ,
    • you can check this video to learn more about accessibility
    • I hope this was helpful for you, other than that, nicely done, happy coding 😄
    0