Skip to content
OverviewSolutions
29
Comments
242
Fer
@fernandolapaz

All comments

  • turboo-sy•250
    @turboo-sy
    What challenges did you encounter, and how did you overcome them?

    The problem that I'm having is that when I click the button , I have to press it twice to go to the next advice Is the api the problem, or there's a different reason for it?

    Advice Generator API

    2
    Fer•3,970
    @fernandolapaz

    Hello, regarding your question:

    On the API site https://api.adviceslip.com you can see the note "Advice is cached for 2 seconds. Any repeat-request within 2 seconds will return the same piece of advice."

    That is the reason.

    I hope this clarifies your doubt.

    Greetings,

  • Fer•3,970
    @fernandolapaz

    Hi, regarding your comment:

    It is a shadow not a border, so you could have used the box-shadow property for that.

    Something like this box-shadow: 8px 8px black;

    I hope it’s useful!

  • Fer•3,970
    @fernandolapaz

    Hi 👋🏻, regarding your comment:

    • You could have used background-color: hsla(178, 100%, 50%, 0.5) and then opacity: 1 on hover, like so:
    .bg {
  position: absolute;
  opacity: 0;
  width: 300px;
  height: 300px;
  border-radius: 10px;
  background-color: hsla(178, 100%, 50%, 0.5);
  display: flex;
  justify-content: center;
  align-items: center;
}

.bg:hover {
  opacity: 1;
  cursor: pointer;
}
    • And remember that when there is a hover state over an element it means that it is interactive, so there must be an interactive element around it (like a link or a button). So, the image should be wrapped with <a> or <button> (depending on what happened when clicking on it), and also 'Equilibrium #3429' and 'Jules Wyvern' should be wrapped with the <a> tag.

    I hope it’s useful : )

    Regards,

    Marked as helpful
  • Fer•3,970
    @fernandolapaz

    Hi 👋🏻, some of this may interest you for future projects:

    • To group a set of related items, like the stats in this case, you could use an unordered list <ul>.
    • It might be good to get used to designing with the mobile-first approach (which means designing for mobile first and then for desktop or any other device) as it is widely considered best practice.
    • As an alternative to color the image you could simply use the mix-blend-mode property:
    .image {
    background-color: var(--clr-primary-soft-violet);
}
.image img {
    mix-blend-mode: multiply;
    opacity: 0.75;
}

    I hope it’s useful : )

    Regards,

    Marked as helpful
  • Fer•3,970
    @fernandolapaz

    Hi 👋🏻, some of this may interest you for future projects:

    • When there is a hover state over an element it means that it is interactive, so it should be an interactive element around it (like a link or a button). So, the image should be wrapped with <a> or <button> (depending on what happened when clicking on it), and also 'Equilibrium #3429' and 'Jules Wyvern' should be wrapped with the <a> tag.
    • In cases like this where the content is centered on the page it is better to use min-height: 100vh for the body as using height causes the page to be cut off in viewports with small height (such as mobile landscape orientation).
    • Consider using rem for font-size as well as it is better for scalable layouts.

    I hope it’s useful : )

    Regards,

    Marked as helpful
  • Fer•3,970
    @fernandolapaz

    Hi Soraya, nice solution,

    Just in case you are not aware:

    The trick of setting font size: 62.5% is no longer commonly recommended, actually there is still a lot of debate about it. In any case, it could be good to get used to thinking directly in relative units since they are the most suitable for most cases.

    More about the 62.5% trick 📗

    I hope it’s useful : )

    Regards,

    Marked as helpful
  • Fer•3,970
    @fernandolapaz

    Hi Dribbz, just a couple of little tips in case you want to take a look:

    • If you use padding instead of margin for the <body> it will avoid the vertical scrollbar you currently have.
    • And if you want to remove the empty space below the image between 600 and 605px (I guess you didn't even see it) you could do it by giving it a height: 100%.
    • Regarding your comment, I personally don't see any problem in terms of semantics/accessibility. 👌🏻

    I hope it’s useful, nice solution : )

    Regards,

    Marked as helpful
  • Fer•3,970
    @fernandolapaz

    Hi 👋🏻, regarding your comment:

    • You could use the mix-blend-mode property that specifies how an element's content should blend with its direct parent background. So, set the soft violet background-color to the image container and then:
    .img-color {
mix-blend-mode: multiply;
opacity: 0.75;
}

    And some of this may also interest you:

    • If you want to remove the empty space below the image between 768 and 940px you could do it by giving it a height: 100%. And if the image stretches too much you can add object-fit: cover.
    • To group a set of related items, like the statistics in this case, you could use an unordered list <ul>.
    • It is better to use min-height: 100vh ('h-screen') as using height causes the page to be cut off in viewports with small height (such as mobile landscape orientation).
    • Think about using relative units since they are better for scalable layouts. Something simple to start with would be to convert to rem (1 rem equals 16px by default), consider this suggestion especially for the font-size.

    I hope it’s useful, and let me know if it worked : )

    Regards,

  • Fer•3,970
    @fernandolapaz

    Hi 👋🏻, regarding your comment:

    You could center the content with grid or flexbox:

    body {
min-height: 100vh;
display: grid;
place-content: center;
}

    body {
min-height: 100vh;
display: flex;
justify-content: center;
align-items: center;
}

    One advantage of this over using translate is to avoid page clipping in viewports with low height (such as mobile landscape orientation).

    I hope it’s useful : )

    Regards,

    Marked as helpful
  • Fer•3,970
    @fernandolapaz

    Hi 👋🏻, some of this may interest you:

    • The trick of setting font size: 62.5% is no longer commonly recommended, and it is good to get used to thinking directly in relative units since they are the most suitable for most cases. More about the 62.5% trick here 📗
    • You should remove the media from <link rel="stylesheet" href="css/styles.css" media="screen and (min-width:375px)">. It is not necessary and it makes the page not have styles below that width.

    I hope it’s useful : )

    Regards,

    Marked as helpful
  • Fer•3,970
    @fernandolapaz

    Hi 👋🏻, some of this may interest you:

    • The image is interactive too, so it should be wrapped with <a> or <button> (depending on what happened when clicking on it).
    • The icons are decorative and therefore need an empty alt attribute to be ignored by screen readers.
    • It is better to use min-height: 100vh for the body as using height causes the page to be cut off in viewports with small height (such as mobile landscape orientation).
    • Think about using relative units since they are better for scalable layouts. Something simple to start with would be to convert to rem (1 rem equals 16px by default), consider this suggestion especially for the font-size.

    I hope it’s useful : )

    Regards,

    Marked as helpful
  • Fer•3,970
    @fernandolapaz

    Hi 👋🏻, some of this may interest you:

    • The image is not decorative and therefore the alt attribute should have at least a short description, like 'QR code to visit Frontend Mentor'.
    • It is better to use min-height: 100vh for the body as using height causes the page to be cut off in viewports with small height (such as mobile landscape orientation).
    • Regarding what you mention about reducing unnecessary code, the only thing I see is that you could remove font-weight: bold from the <h2>.

    I hope it’s useful : )

    Regards,

    Marked as helpful
  • Fer•3,970
    @fernandolapaz

    Hi 👋🏻, this may interest you for future projects:

    This challenge consists of a group of interactive elements where users are expected to enter data (their rating) to be sent.

    Therefore, it would be appropriate to use a <form> (whose inputs should be radio buttons <input type='radio'> so that it is only possible to select one option).

    And a <button type=' submit'> (<input type='submit'> is fine too) to send that data.

    More about Forms here 📗

    I hope it’s useful : )

    Regards,

  • Fer•3,970
    @fernandolapaz

    Hi 👋🏻, this may interest you for future projects:

    Theoretically, in this challenge we have a group of interactive elements where users are expected to enter data (their rating) to be sent.

    Therefore, it would be appropriate to use a <form> (whose inputs should be 'radio buttons' <input type='radio'> so that it is only possible to select one option).

    And the <button type=' submit'>, as you did 👌🏻, to send that data.

    Remember that buttons are interactive elements used to perform an action (such as submitting a form), but to collect user data we should use inputs.

    I hope it’s useful : )

    Regards,

    Marked as helpful
  • Fer•3,970
    @fernandolapaz

    Hi 👋🏻, this may interest you for future projects:

    In theory, here we have a group of interactive elements where users are expected to enter data (their rating) to be sent.

    So, you were right to use <form>👌🏻 but a form is used to collect user input and should be a container for some kind of input elements (not <li>). In this case it would be appropriate to use 'radio buttons' <input type='radio'> so that only one option can be selected.

    And a <button type=' submit'> (<input type='submit'> is fine too) to send that data.

    I hope it’s useful : )

    Regards,

    Marked as helpful
  • Fer•3,970
    @fernandolapaz

    Hi 👋🏻, some of this may interest you for future projects:

    • Headings shouldn't be used to make text big or bold, that's what CSS is for. Also, using more than one <h1> is allowed but is not considered a best practice, so if you are going to use more headings maybe it would be better to use an h2 and so on.
    • It could be better to use the <del> element for the old price because the way you did it, a screen reader user won't know it has a strikethrough.
    • Consider using some CSS reset as a good practice to have a clean starting point and reduce differences between browsers. An example of CSS reset here 🔎

    These are some very basic and useful ones to start:

    * {
box-sizing: border-box;
margin: 0;
padding: 0;
}
    • Think about using relative units since they are better for scalable layouts. Something simple to start with would be to convert to rem (1 rem equals 16px by default), consider this suggestion especially for the font-size.

    I hope it’s useful : )

    Regards,

    Marked as helpful
  • Fer•3,970
    @fernandolapaz

    Hi 👋🏻, some of this may interest you for future projects:

    • Skipping heading levels can be confusing and should be avoided whenever possible (start from <h1>, followed by <h2> and so on).
    • The icons are decorative and therefore need an empty alt attribute to be ignored by screen readers.
    • When there is a hover state over an element it means that it is interactive, so there must be an interactive element around it (like a link or a button). So, the image should be wrapped with <a> or <button> (depending on what happened when clicking on it), and also 'Equilibrium #3429' and 'Jules Wyvern' should be wrapped with the <a> tag.
    • You could also use flexbox to center the content of the page:
    body {
min-height: 100vh;
display: flex;
flex-direction: column;
justify-content: center;
align-items: center;
}

    And removing the margin from the 'main-container'.

    • Think about using relative units since they are better for scalable layouts. Something simple to start with would be to convert to rem (1 rem equals 16px by default), consider this suggestion especially for the font-size.

    I hope it’s useful : )

    Saludos,

    Marked as helpful
  • Fer•3,970
    @fernandolapaz

    Hi 👋🏻, this may interest you for future projects:

    The design looks good 👌🏻, but keep in mind that the main thing about this challenge is that we have a group of interactive elements where users are expected to enter data (their rating) to be sent. Therefore, it would be appropriate to use a <form> (whose inputs should be 'radio buttons' <input type='radio'> so that it is only possible to select one option). And a <button type=' submit'> (<input type='submit'> is fine too) to send that data.

    I hope it’s useful : )

    Regards,