Skip to content
OverviewSolutions
3
Comments
11
Marvin Baga
@mjbaga

All comments

  • Marvin Baga•310
    @mjbaga

    Hi, @tab21. Nice job on the card. It looks great.

    I've checked the code and see that you're using a background image. I also suggest try doing alternate one where you're using an image tag. This might not be useful now but typically on the job, the image tags are the way to go because:

    • Screen readers (assistive technology) won't be able to read background images on CSS. They can read image tags and their alt attributes.
    • Sometimes when working with backend and the image source comes from them, they can't place the image unless they use a style tag and some policies like CSP prevent inline styles.

    Otherwise, solution is great. Hope this helps. Happy coding.

  • Marvin Baga•310
    @mjbaga

    Hi Ali. Nice work on the challenge.

    Just some feedback:

    • Was already pointed out on the social media icons. This usually happens when height and width doesn't match. You can try setting an explicit height and width, maybe in pixels. Or you can define just the width or height and use aspect-ratio: 1/1;
    • I'm on a larger screen and the social media icons are cut off, almost by half and can't scroll down. Maybe doesn't need the overflow: hidden on body.
    • On your button, maybe you can add a small transition effect, transition: color 300ms ease-in;. These are micro-interactions that improve user experience.

    Hope this helps! Happy coding!

    Marked as helpful
  • Marvin Baga•310
    @mjbaga

    Hi, Emma. Good work on the slider!

    Nothing much to say except maybe a few ideas on transition of the slides. Small animations just make the user experience better. Some things you can try:

    • Slide animation, usually you declare a long width so that you can place your slides side by side and use overflow-hidden to hide that slide. Then you can play with transform translate rule and use transition. Or even use animate CSS rules.
    • Fade in, fade-out, is much easier, you just stack the slides up and fade in the the new slide.

    Oh, lastly, maybe a better hover effect than shadow on the buttons. Maybe background change instead.

    Hope this helps! Happy coding.

    Marked as helpful
  • Marvin Baga•310
    @mjbaga

    Hi @topspinppy. Good job on the challenge.

    I've checked your code and have a few feedback:

    • I would recommend to separate your media queries in big group chunks, instead of repeating the media query includes on a few declaration blocks. It's something I also used to do, but when you check your compiled styles.css, you'll see the same media queries were repeated like 16 times on the file. It wouldn't matter much on very small projects but quite significant or large ones, even saving a few kilobytes.
    • You can check out mobile-first development. It helped me out a lot in coding for responsive design. Basically it's coding for mobile first and adding media queries as you go up in screen size. So you'll be using your min-width mixins instead of max-width.

    That's it. Hope this helps! Happy coding!

  • Marvin Baga•310
    @mjbaga

    Hi, Niezzx! Good job on the challenge.

    I've checked your code out and here's some feedback:

    • The container is expanding because of the hidden paragraph content being shown. Notice that it doesn't expand when the paragraph isn't as long as 2 lines. Try transferring that hidden paragraph inside the question-bar div so that it will follow the width of its parent container and you won't have to us percent width on the paragraph.
    • I checked your JS. For adding event listeners to querySelectorAll, you don't need to loop through each item. I get that you're trying to access the ID, but you can just do it by simply traversing the DOM.
    • Check out this article https://medium.com/codex/how-to-traverse-the-dom-in-javascript-7fece4a7751c for traversing DOM. Basically if you have queried an element, like with querySelectorAll, you can use that as a basepoint to either go up the DOM to access parents, or go down the DOM to access children.

    Hope this helps! Happy coding!

    Marked as helpful
  • Marvin Baga•310
    @mjbaga

    Hi Devbev. Good job on the challenge.

    Just some feedback, just very small things:

    • Just semantic HTML, suggest to use <ul></ul> for lists, items that are related with each other. To divide stuff into sections for layout, use divs instead.
    • Font sizes are smaller and the colors for Equilibrium and Jules Wyvern are different from design.

    That's it! Otherwise, nice work. Hope this helps.

    Marked as helpful
  • Marvin Baga•310
    @mjbaga

    Hi, Nguyen. Good job on the challenge.

    Your questions:

    1. I'd use grid as well, much easier with grid-template-columns/rows or grid-template-areas.
    2. I suggest maybe add a transition effect on your hover for nicer micro-interactions. Something like transition: box-shadow 500ms ease-in-out;. Also maybe less shadow spread so it doesn't cover up any other elements (unless intended).

    Hope this helps. Happy coding!

    Marked as helpful
  • Marvin Baga•310
    @mjbaga

    Hi, Jessica. Good work on the challenge.

    I've checked your code and here's some feedback:

    • Try adding a container for max-width on the breakpoints so that the site won't be very stretchy, especially when reaching very wide screens.
    • For file organization, maybe try breaking down your header and footer on their own components.
    • You can look into mobile-first styling for handling responsiveness. It's a concept where you style out your mobile CSS rules then add media queries as you increase screen size like for tablet or desktop.
    • For this last one, just a personal preference (you can ignore haha), I'd say either just go full Sass or full utilities library, but I'd go with Tailwind as it's really more flexible than bootstrap, especially with responsiveness.

    That's it. I've also done Designo, you can check it out. Hope this helps and happy coding!

    Marked as helpful
  • Marvin Baga•310
    @mjbaga

    Hi, Monica. Card looks awesome. Nice job.

    Tip for media queries, maybe you can look into mobile-first styling. So it's concept where you set your css rules for mobile first, then when there's rules that change as you go higher like tablet and desktop, you do your media queries then. Really helped me a lot in doing responsive designs.

    Marked as helpful
  • Marvin Baga•310
    @mjbaga

    Hi, Wayne. Great job. The good thing about CSS is there's lots of ways to approach things. I would have probably added padding-inline on your text-container, but reducing width also works well.

    Some feedback, for the image, I tend to not be explicit on the width in pixels, especially when parent width is already defined, just have the image try to follow the parent width with 100%.

    Anyway, nice job. Happy coding.

    Marked as helpful
  • Marvin Baga•310
    @mjbaga

    Hi, Mateus. You can centralize elements inside a grid or flex vertically and horizontally using "place-items: center;". It's the short version of "justify-items: center;" and "align-items: center;".

    For flex, depends on which direction if it's column or row, justify-items or align-items.

    For non-grid or non-flex, try "vertical-align:" sub or middle, etc. You'd have to manually check.

    Last would be just adding padding on inline elements.

    Marked as helpful