Skip to content
OverviewSolutions
19
Comments
22
David Osorio
@osoriodev

All comments

  • David Osorio•560
    @osoriodevPosted almost 2 years ago

    Hi Ali

    There's a problem. When you deploy a project with GitHub Pages, the root directory(/) refers to username.github.io, not the repository itself. You must use a relative path, in this case it must be body { background-image: url("../images/bg-desktop.svg"); }

    Btw, remember to use the aria-label attribute for <a> tags when there is no discernible text.

    I hope it helps.

    Marked as helpful
    0
  • Joshua Daniels•50
    @JoshuaADSubmitted almost 3 years ago

    I attempted to complete this site while using a figma file. Should I use the absolute positioning that is stated in the figma file or should I use my own judgment about flexbox, grid, etc. This is my first time making a website while having a figma to go off of. So it's very new to me. Feedback would be very much appreciated.

    Used Absolute positioning

    1
    David Osorio•560
    @osoriodevPosted almost 3 years ago

    Hello @JoshuaAD

    The best thing you could do is work with your own styles. It's true that Figma provides certain styles, but they are not actually optimized to work on a real website. You can use Figma to check for more specific things, for example: font sizes, colors, paddings, widths, etc.

    Btw, you have some accessibility issues. There must be an h1 on the page, it's very important for semantic and accessibility reasons.

    0
  • David Osorio•560
    @osoriodevPosted almost 3 years ago

    Hello @titocs, you have some accessibility issues.

    • You are using an h1 for each card title. There can only be one h1 on the page.
    • You have an h3 for the person's name and an h5 for the text "Verified Graduate", that doesn't make sense.

    When you use HTML headings, you must follow a logical order like a book's index.

    • h1
      • h2
        • h3
        • h3
        • h3
      • h2
      • h2

    Headings are usually separated by an article, a section, etc. I hope it helps you 👍

    0
  • David Osorio•560
    @osoriodevPosted almost 3 years ago

    Hello. I see your styles don't work. <link rel="stylesheet" href="/styles.css">

    You must add a dot to the href: <link rel="stylesheet" href="./styles.css">. This is to indicate that the file is on the same path, the same for the images.

    Btw, remember that there must be an h1 on the page.

    Marked as helpful
    1
  • David Osorio•560
    @osoriodevPosted almost 3 years ago

    Hello Obuhri.

    Your solution looks great. As a suggestion you can use a tags for the social media icons, I think it is more appropriate than using a span. I hope this helps ✌

    1
  • Briuwu•750
    @BriuwuSubmitted almost 3 years ago

    Hello~! (≧∇≦)ﾉ

    For this project, I'm still adjusting on using JavaScript and CSS animation, but so far I think I'm improving little by little! But I know that it's still not enough so I will do my best!

    If you have any feedback or suggestions, please do let me know! Thank you so much.

    Responsive Article Preview Component

    3
    David Osorio•560
    @osoriodevPosted almost 3 years ago

    Hello there.

    I suggest you use a button element for the share option and put the icon inside, same for social media icons, it is better to use a tags and put the icons inside. This for better accessibility and semantic HTML.

    Don't forget to use the aria-label attribute as there is no discernible text.

    2
  • David Osorio•560
    @osoriodevPosted almost 3 years ago

    Hello @Saran-73

    I was checking the website and I see that you set the main with grid-template-rows: 40% 50%, but you didn't set a specific high, so the percentages don't know what value to refer to. Maybe this is the reason why an bug occurs and this property is ignored.

    In fact, if you remove that property, the website will look the same as the screenshot. I suggest you remove that property and modify the padding of the sections below.

    Marked as helpful
    1
  • David Osorio•560
    @osoriodevPosted almost 3 years ago

    Hello 👋

    A couple of things that I can recommend you

    • Try not to use <br> tags, since these modify the document flow and that must be done from the CSS. If you want to create a spacing, you can use the margin properties.
    • Use more semantic HTML, for example Sedans, Suv and Luxury are titles, in this case you can use HTML title tags, the span is for text that has no relevance.

    Overall, your project is a good start. Keep coding 👨‍💻

    Marked as helpful
    1
  • Clarice Almeida•65
    @ClariceAlmeidaSubmitted almost 3 years ago

    This was my first challenge using JavaScript, please feel free to give feedbacks.

    Responsive site

    1
    David Osorio•560
    @osoriodevPosted almost 3 years ago

    Hello @ClariceAlmeida 👋

    You have some issues with your HTML, for example:

    • You are using an a tag to send email, it must be a button, the a tag is for links only
    • The <div id="text"> must be an h1 tag
    • The text tag is not a valid html tag, you can use other tags instead, for example span

    As for JavaScript, I recommend that you use regular expressions for validation.

    Overall, your design looks great. I hope it helps. 👍

    Marked as helpful
    0
  • Victor-Gelado•220
    @VictorGeladoSubmitted almost 3 years ago

    Any tip would help a lot, I'm a beginner and would like to gain experience. The overflow: hidden; is not working on mobile, if anyone knows how to disable scrolling on mobile, it would help a lot.

    Used HTML5 and CSS3, flexbox.

    1
    David Osorio•560
    @osoriodevPosted almost 3 years ago

    Hello @VictorGelado 👋

    To solve the problem, you can do this:

    • Add position: relative to the wrapper, this way the images will be positioned with respect to this element
    • It also changes the attribution styles for these, so it looks good on mobile devices.
    .attribution {
  width: 100%;
  position: absolute;
  left: 0;
  bottom: 4px;
  text-align: center;
  font-size: 16px;
  color: #fff;
  z-index: 4;
}

    I hope I've helped 👍

    0
  • David Osorio•560
    @osoriodevPosted almost 3 years ago

    Hello @akinsanyajoseph 👋

    Your solution looks good. I see that the eye icon is not centered properly, you can do this to fix it:

    .overlay-icon {
  position: absolute;
  top: 50%;
  left: 50%;
  transform: translate(-50%, -50%);
}

    With this it should work fine.

    Marked as helpful
    0
  • David Osorio•560
    @osoriodevPosted almost 3 years ago

    Hello @ArnasLuksas 👋

    I was reviewing your code and I see that you are using a validation with includes(). In this case it would be appropriate to use regex (regular expressions) for validation. In short, a regular expression is a sequence of characters that forms a search pattern. For example, a regex for the email can be:

    const re = /^[a-zA-Z0-9_.-]+@[a-zA-Z0-9-]+\.[a-zA-Z]+$/

    If you want to know if an email is valid, you can use the test() method, this will return true or false.

    re.test("[email protected]") // true
re.test("@example") // false

    If you want to know exactly how characters work, you can find a longer article on the subject here: Regex

    A couple of tips on accessibility

    • Since there is no text in the email button, you must to use the aria-label attribute, for example: aria-label="Send email"
    • Use the appropriate input tag, in this case <input type="email">

    I hope it helps you. 😁

    Marked as helpful
    0