Skip to content
OverviewSolutions
17
Comments
70
Marlon Passos
@MarlonPassos-git

All comments

  • Marlon Passos•920
    @MarlonPassos-git

    hey bro. i try to acess the site but. Always when i try start a looping when is not possible for me finish the register

  • Marlon Passos•920
    @MarlonPassos-git

    hi John, nice solution.

    I really liked the way you used to style the hover color of the social icons. What was your strategy to get to that role and the color is the same as the designer?

  • Marlon Passos•920
    @MarlonPassos-git

    Hi Dave, great job as always. Flexible, accessible, functional and very beautiful. I really liked the care you gave when the user has JS disabled (It doesn't happen much but it's good to be careful). It becomes more and more difficult to find something wrong 😅. However I wanted to point out 2 little things that I noticed.

    .that shortcut at the beginning to skip navigation was a good thing, but I tried to access it through the tab "Speedy Searching" and "Easy Sharing" and it wasn't possible, it goes straight.

    • I liked that when I access via Tab the buttons also activate the hover effect, it would be nice if the elements of footer and "Ask" also activated.

    Keep up the good work 👏

    Marked as helpful
  • Marlon Passos•920
    @MarlonPassos-git

    Wow this project was very cool, it must have been a lot of work. Congratulations man. It really looks good, flexible, functional and ok with the original version. I don't know anything about the framework, so I don't have much to suggest, but a few things I saw and I'm going to point out.

    • In the form on the phone part it is accepting letters as something valid, I recommend you leave only numbers

    • both "3886 Wellington Street Toronto, Ontario M9C 3J5" and "Australia" "USA" could be <adress> tags make more semantic sense

    • "OUR COMPANY LOCATIONS CONTACT" should be inside a <nav> tag

    • Items inside "App Design; Web Design Graphic Design" are not accessible by TAB, this is not very good for accessibility

    • The tablet version in the location area is very strange, I don't know if that's how it was in the designer, but I found it weird. I would increase the size of the map, make it centered or something. See the screenshot https://prnt.sc/1vwev9g

    • Where you put phone numbers you could have used a <a href="tel:+496170961709"> tag that would cause the user to click would already take them to make a call

    • Put <input type="number"> in the phone form to show only numbers on the android keyboard

    • And type="email" in the email part to pop the @ easily for the mobile user

    • There are a lot of accessibility errors in your project report, it would be nice to read it, we always learn something new there.

    Again congratulations for the project was very good. (I hope to reach this level someday 😅)

  • Marlon Passos•920
    @MarlonPassos-git

    Good job,

    some suggestions

    • "Proceed to Payment" could be a <a> tag
    • It would decrease the font size a little
    • Increase the size of the card a little
    • change the repeat to background-repeat: repeat-x; because in this way the background breaks on big screens
    • Add a hover effect to "cancel" and "change"
    • Change your card padding to clamp(1.25rem, 0.8099rem + 1.8779vw, 2.5rem);, this would automatically change the value from 20 to 40px flexibly and without media query
    Marked as helpful
  • Marlon Passos•920
    @MarlonPassos-git

    Congratulations on your completed 10 Challenge 🎉🎉🎉

    I really liked your solution, flexible, functional and similar to the original version.

    However I have some suggestions for you to improve even more.

    • "Terms and Services" should be an <a> tag as this usually takes the user to another page

    -As a good practice and to avoid future bugs, I recommend putting the script tag just before the closing </body> tag rather than in the <head> section of your HTML, any doubt I recommend reading https://levelup.gitconnected.com/all-about-script-87fea475b976

    • Another nice thing to do is enable the hover effect when using TAB, just add , :focus-visible to your hover CSS. Example
    .intro__button:hover, .intro__button:focus-visible {
        cursor: pointer;
        opacity: 0.65;
 }

    • When I get out of 750px width the designe shrinks at once, I think if you set a maximum width in the form and only changed the button it would leave a lot more space for the user to interact.

    • About the title, there is a really cool technique where you could make the font size of the title flexible media query using clamp(), I recommend you read this article https://css-tricks.com/linearly-scale-font-size-with-css-clamp-based-on-the-viewport/#for-those-who-dont-mind-that-edge-case

    Marked as helpful
  • Marlon Passos•920
    @MarlonPassos-git

    Good job Murilo.

    Some suggestions:

    • I would recommend you break the layout a little earlier, as there are times when elements get very shrunk

    • In the profile pictures I would add the alt of the picture with the person's name

    • I would change the line-height to 1, so it will always be relative to the font size it will not be necessary to change with @media

    • would add background-position: center top, right bottom; so it would be the same as the original version that the images are pasted in the corners and not centered

    Marked as helpful
  • Marlon Passos•920
    @MarlonPassos-git

    congratulations for your first challenge, some suggestions: Add this code in the body to center the card min-height: 100vh; display: grid; place-intens: center; .

    • Each of the sections could be inside a <section> tag and all the content could be inside a <main> tag, this would make the code more semantic and organized.
    • Seeing that you styled css using standard HTML tags, I would recommend you always use classes to style as this helps to avoid errors in the future
    Marked as helpful
  • Marlon Passos•920
    @MarlonPassos-git

    I really liked its very flexible and smooth implementation.

    Some suggestions:

    • When I click on the Wikipedia link, and then use the browser's back button to your 404 error page https://prnt.sc/1vp648o, it would be nice for you to solve this, many users do this type of navigation

    • Instead of the images breaking from 2 columns to 4 direct columns, it would be nice to break down to 3 columns as well,

    • It would be nice to add some keyboard commands, arrows to switch images, ESC to go back to menu, ENTER or SPACE to open image, this always brings a good experience for the user.

    • I could be wrong, but I think when we click on "start slide show" it should make the images change themselves, if not blz, but that would be something cool would be 😁

    • menu-container could be in a <header> tag and galleria-container could be in <main> and slide-show-playlist could be a <footer>

    • close and view the images should be a button to be accessible to the keyboard

    • Instead of creating a div for each element you could have left all the elements loose and atomically they would be organized by some more advanced properties of grid or flex. I would recommend you research how pinterest organizes its images

  • Marlon Passos•920
    @MarlonPassos-git

    Hi Al-Baraa, some suggestions for you to improve your project

    -I would suggest for you to add a stronger style to the clickable button so that it's easier for the user to know that there are more images, maybe adding a cursor: not allowed for disabled button would be nice.

    -Another thing I see on sites with carousel is the possibility to change the image using the keyboard arrows, maybe you can implement that too

    • Add a larger margin to the image

    • The arrows should have no <button> tags as it is currently inaccessible by TAB

    • And add the background images, I think you must have forgotten 😅

    Marked as helpful
  • Marlon Passos•920
    @MarlonPassos-git

    some suggestions

    • each box I would put inside a <section>

    • I would put this code in an assignment so that it stays in the corner (decktop version, on mobile it would be something else)

         position: absolute;
     bottom: 15px;
     right: 20px;

    • I would reduce the size of the cards for the mobile version, they stretch a lot.

    • Instead of using padding: 15% on boxes I would recommend you to use something more FLEXIBLE 😙, I strongly recommend you to read this article, you will learn a good trick https://css-tricks.com/linearly-scale-font-size- with-css-clamp-based-on-the-viewport/#for-those-who-dont-mind-that-edge-case

  • Marlon Passos•920
    @MarlonPassos-git

    Very cool scrool animations, is there a way to do this only with css? some suggestions:

    • You could have organized your content in the <header> <main> <footer> tags it would have been much more legive.

    • I would remove the width: 100% of the image__container as it has on some screen sizes it is inconsistent, see https://prnt.sc/1vmt6xk

    • "What is it?" it should be an <a> tag as it will take the user to another page and this makes more semantic sense

  • Marlon Passos•920
    @MarlonPassos-git

    hello suraj, some suggestions for your project:

    • Put the backgrounds on the body and use background-size: cover; to cover the entire desktop area and background-size: contain; in the mobile version
    • You could break to mobile version a little earlier, there are times when your image gets too small
    • Add a border to social network elements to look more like the original version
    • The logo could be inside a <header> tag and the content of the image and texts inside a <main> tag
    • The footer should be at the bottom of the page, you can solve this by adding a grid to the body, and setting the footer to justify-self: end; align-self: end, if you have any doubts on how to do this take a look at this solution https://huddle-landing-page-with-single-introductory-section-96lkg2nms.vercel.app/
    Marked as helpful
  • Marlon Passos•920
    @MarlonPassos-git

    Hi, I don't have much to say about your project, it was very consistent. What I would recommend would be the following:

    add background color, i think you forgot 😅

    • I would put this code in the body to center the container: ```display: grid; place-items: center; min-height: 100vh;
    Marked as helpful
  • Marlon Passos•920
    @MarlonPassos-git

    I really liked your solution, it must have been a lot of work, some suggestions:

    • users are kind of used to clicking on the website logo and being directed to the home, it would be nice for you to implement this
    • Another thing that would be nice is to click on the sides when selecting the product and automatically open the card
    • You could have hidden the content using a Details & Summary tag, it would be more sematic and maybe even easier to work with.
    • locations and social could be a <ul>
    • And each location could be an <adress>
    Marked as helpful
  • Marlon Passos•920
    @MarlonPassos-git

    Very cool your solution, practically a pixel perfect.

    particularly the only thing I could suggest to you is to add a stronger style to the clickable button, so it's easier for the user to know that there are more images, maybe adding a cursor: not-allowed to the disabled button would be nice. Another thing I see on sites with carousel is the possibility of changing the image using the keyboard arrows, maybe you can implement that too 😄

  • Marlon Passos•920
    @MarlonPassos-git

    Very nice your joaquin solution, very similar to the original version, but I have a couple of semantic suggestions for you

    • lodon could be inside an <address> tag

    • And in my view the profile footer data could be inside a <ul> list

    Marked as helpful
  • Marlon Passos•920
    @MarlonPassos-git

    some suggestions:

    • Study a little about semantics with html, I see you use many divs, many of them could be replaced by <section> <main> <footer> tags. For example, all card content should be inside a main tag and your FEM information inside the footer tag.

    • On body you could have used the following code to center the card

        place-items: center;
    display: grid;
    min-height: 100vh;
    • to put the right background image could have done like this
        background-size: contain;
background-color: #e0e8ff;

    • It was not necessary to border the card

    • In the footer I would put this code to always leave your information right at the bottom and still without interfering with the grid positioning

        position: absolute;
    bottom: 15px;
    • In the plan add a margin and a curve at the edge
    border-radius: 20px;
    margin: 0 30px;

    • On the image instead of writing border-top-right-radius: 25px; border-top-left-radius: 25px; type border-radius 25px 25px 0 0 . Much easier to understand and shorter

    • add a curve on the edge at the bottom of the card

    -Add a hove effect to the "share" and cancel button

    • Only with these suggestions I gave, look how the design would look, https://prnt.sc/1vm3zra

    • I would recommend you take a look at other people's solutions to see how others have done so you can learn a lot

    Marked as helpful