Skip to content
OverviewSolutions
33
Comments
38
Carlos
@Carlos-A-P

All comments

  • Carlos•1,110
    @Carlos-A-PPosted about 3 years ago

    Hello, Style-wise, this looks great and responsive.

    • Something that I noticed was that the first accordion in mobile view is unclickable unless I tab to it. It looks like it was a height issue with your .card_visual-content div. Something I did differently was adding position relative to my card-wrapper div and added position absolute to my child icon div. This allowed me to move the child freely around the parent div. I also added padding-top and a background image of the shadow
    Marked as helpful
    1
  • Carlos•1,110
    @Carlos-A-PPosted about 3 years ago

    Hola Enmanuel, este parece un buen proyecto! Algo que miraría sería:

    • Tal vez solo permitir números enteros en el número de personas.
    • Para el output, algo que hice de manera diferente fue usar una <p> tag y cambiar el innerHTML con JavaScript
    • Estos son algo que puede considerar, pero en general se ve bien.
    Marked as helpful
    1
  • Carlos•1,110
    @Carlos-A-PPosted about 3 years ago

    That was a nice intro!

    • I noticed that between 750px and 1260px I'm not able to view a few of the cards.
    • I see that you used similar id's a couple of times. To my understanding, id's are single-use and are only applied to one element. Each element is attributed as a unique identifier to (only) one single element so I think that's one reason why the report shows multiple errors
    0
  • Carlos•1,110
    @Carlos-A-PPosted about 3 years ago
    • One thing I'd look into would be the input functionality. It's a bit difficult to use for me. When I'd use the keyboard to move right, the text cursor would move to the left and vice versa.
    • The number I type also doesn't appear next to the text cursor when I'm hovering over the beginning or end of my input.
    2
  • Carlos•1,110
    @Carlos-A-PPosted about 3 years ago

    Hello,

    • So I noticed that you have some margin of 8px on your body element so you can remove that with margin: 0;
    • Also you could change the styling of the input element's background and border instead of adding a background and border to the form element. Then one way you can move the button into the input is with absolute positioning. Hope this helps and happy coding!
    Marked as helpful
    0
  • EDllT•60
    @EDLLTSubmitted about 3 years ago

    Hey! So, I feel like I've really messed this one up, as I still don't have a solid grasp on how to make responsive layouts

    However, any feedback on how to improve would be appreciated. Thank you!

    Raw CSS & HTML

    2
    Carlos•1,110
    @Carlos-A-PPosted about 3 years ago

    Hey Edllt,

    • So a way to make this project responsive is by using media queries. You can use
    @media (max-width: 900px) { 
/////////code
 }
    • For the image, you can use background-blend-mode to set the color to the image. This is an example from my code:
    .image {
    min-height: 250px;
    border-radius: 0.75em 0.75em 0 0;
    background-image: url(../images/image-header-desktop.jpg);
    background-size: cover;
    background-repeat: no-repeat;
    background-blend-mode: soft-light;
    background-color: rgb(112, 51, 142);
}
    Marked as helpful
    1
  • Carlos•1,110
    @Carlos-A-PPosted about 3 years ago

    Hello Norrico,

    • There seems to be some overflow in your .content div. You can maybe use media queries to make the layout verticle around 980px so that the text has more room.
    • You can also remove your padding-right from your .likes div so that the child elements are centered.
    Marked as helpful
    0
  • Christine C•30
    @oemorbSubmitted about 3 years ago

    Hi everyone,

    Thanks for checking my code out! I'm pretty happy about this one.

    If possible, I'd like to get some feedback if there's a neater way of making the "Learn More" text in the button have the background of the box and not the actual button.

    Also, if there's a neater way of making the border-radius change in the container element, I tried but it didn't have an effect.

    xoxo

    Christine

    Flexbox CSS HTML :hover

    1
    Carlos•1,110
    @Carlos-A-PPosted about 3 years ago

    Hey Christine,

    • It looks like after the screen reaches 1040px the .container child items each have different widths, you can fix that by adding a max-width to your .container.
    • Your .container should also have a width: -moz-fit-content; along with width: fit-content; which is why the compared images look different. Firefox doesn't support width: fit-content; and needs the prefix.
    0
  • Mathis•770
    @MathisHumbertSubmitted about 3 years ago

    This is an old project that I did 2 months ago. My javascript is not the best but the app is working.

    Feel free to comment my code!

    Todo App Main using HTML, CSS and JS

    1
    Carlos•1,110
    @Carlos-A-PPosted about 3 years ago

    This looks great and responsive, good job!

    • You could use button tags for the bottom buttons and maybe add a highlight to the button the user selected to indicate which filter they chose.
    • You can also use localhost to save the user's theme and list items. You can learn more about it here
    • The report also shows other suggestions to improve accessibility, which seems like you need to include alt attributes to your images. Other than that it looks great.
    0
  • Carlos•1,110
    @Carlos-A-PPosted about 3 years ago

    Hey Joel, a few things I noticed:

    • Style-wise, the font is a little small going into desktop view, other than that the layout looks great!

    Some suggestions:

    • The theme doesn't seem to save when I refresh the page, you can use local storage to save the users theme. You can learn more about local storage here

    • Here's an example from my todo project of how I used local storage:

    • I used the onclick event attribute but you can also use addEventListener instead

    <button onclick="toggleDark()"></button>

    //--------------------------theme changer
function toggleDark() {
   // if user toggles light-theme, remove dark theme and save light theme in local storage
  if (body.classList.contains('dark-theme')) {
    body.classList.remove('dark-theme')
    localStorage.setItem("theme", "")
  } else {
   // if user toggles dark-theme, add dark-theme class and save dark theme in local storage
    body.classList.add('dark-theme')
    localStorage.setItem("theme", "dark-theme")
  }
}

//if item is set to dark theme, add dark-theme to body
if (localStorage.getItem("theme") === "dark-theme") {
  body.classList.add('dark-theme')
}
//----------------------------------------

    I also used local storage to also save my list-items div in a local storage item. I hope this helps and good luck coding!

    Marked as helpful
    0
  • Carlos•1,110
    @Carlos-A-PPosted about 3 years ago

    Hey Martha, this looks really good and I like how it's responsive

    • Something I did a bit differently was instead of adding a ::before pseudo element to add another shape and change the opacity, I used background-blend mode and set my image as a background image:

    <div class="image"></div>

    .image {
    background-image: url(../images/image-header-desktop.jpg);
    background-size: cover;
    background-repeat: no-repeat;
    background-blend-mode: soft-light;
    background-color: rgb(112, 51, 142);
}
    Marked as helpful
    0
  • Carlos•1,110
    @Carlos-A-PPosted about 3 years ago

    Hey Norrico,

    • There seem to be fixed widths throughout your CSS for the desktop view which isn't responsive friendly. I'd suggest using max-width instead.

    • As for the accessibility issues, instead of using a <section> tag you should use a <main> tag

    • You can remove display flex and fixed width/ height from your body tag, then in your <section> tag use:

    .container {
    width: 100vw;
    height: 100vh;
    display: flex;
    justify-content: center;
    align-items: center;
    background-image: url(./images/pattern-background-desktop.svg);
    background-size: 100% auto;
    background-repeat: no-repeat;
    /* box-shadow: 0px 16px 28px 0px rgb(0 0 0 / 20%); */
}
    • This actually made it more responsive aha.

    Hope this helps and good luck coding!

    Marked as helpful
    1