Community Feedback

  • Raymart Pamplona has commented on Luiggy Macias's "CSS GRID, FLEXBOX, media queries, responsive web design" solution

    0

    Hey, good work on this one. The layout in desktop is good and the mobile state as well. Though there are couple of suggestions

    1. Font. I think you are rendering the wrong font in here, since the original uses a sans-serif type and you are using a serif type.

    2. Horizontal scrollbar. When resizing, a horizontal scrollbar appears at the bottom which we do not want. It was caused by the declaration of width: 80.5% in your header section selector. Try to remove that and it will remove the scrollbar. Though when you use that, you need to adjust as well the sizing of the image inside it.

    Regarding your query. Well the left pattern will not stay in place, because the container which is currently contained, have a scaling sizes. Meaning the container will resize so is the left background pattern. But if you made the container have a fixed size, then the pattern will stay in place.

    If you need help, just ask here okay and you work is good as well^

  • Raymart Pamplona has commented on Jay's "url-shortening-api-master" solution

    0

    Hey, just here to say that your solutions is really awesome!

    That corner image animation is really clever and those custom hover state in the circles is really good.

    The api works well and that shaking while loading, really awesome. Layout resizes good when going in mobile state and this solution is definitely a good good one^^

  • Raymart Pamplona has commented on Tech Hacks and Tricks's "Profile Card Challenge Using HTML and SCSS" solution

    0

    Hey, great work on this one.

    Layout looks good in desktop and in mobile state as well.

    Though when opening up dev tools, your layout gets squished in the middle, and I suspected, height: 100vh is declared in your bg selector.

    Instead of that, use min-height: 100vh which will be a lot better. But still, your layout won't work because you absolute the card selector which I think not the right choice in here. You want it to be captured by the parent, so making it static or relative is the betters ones.

    So in your card selector. Remove this properties

        position: absolute;
        transform
        left and top

    And in your bg selector. You want to add something like

        align-items: center;
        display: flex;
        justify-content: center;
        padding: add a padding to the top and bottom;

    This will make your content properly aligned in the center while making element in the flow which is a lot better.

    If you need help regarding those suggestion, just drop it here okay and i'll help you with it^

  • Raymart Pamplona has commented on Alex Daniel's "Job listings with filtering created using React, Redux and Sass" solution

    0

    Hey, your solution looks good and it functions very well.

    Resizes well when going in mobile state which is good, one thing to suggest is that. Maybe change the background-color of the filter items, because right now, it is the same with the overall background-color and that box-shadow is the only thing that is separating it.

    Changing that to white like the original will be awesome, so that it will be contrasted well and pop out a bit more.

    But still, your work is really awesome^^

  • Raymart Pamplona has commented on ANIKET PITHADIA's "Pure HTML CSS" solution

    0

    Hey, great work on this one. The illustration looks awesome.

    Though many issue when a user starts to resize the browser.

    • Text is getting hidden via the viewport's width
    • Illustrations if getting pushed to the left which is also getting hidden.
    • There are two scrollbars at the mobile view, which is not really good.

    Also, I saw that you declared height: 100vh in your container selector. Well this works, but opening up dev tools, your layout gets cut out. So it will be awesome if you could remove that. Using min-height: 100vh I think will be the right declaration in here.

    Fixing those mentioned above will be awesome, and while you're it, clearing some issues will be really good ^

  • argel omnes has commented on Adam's "3 Column Card using Tailwind" solution

    0

    Hey Adam,

    I've used Tailwind and I totally agree with your second sentence. The buttons are jumping off because they don't have a border in their default state as mentioned by @pikamart. You can remove the hover prefix to solve this. I have a different suggestion for keeping the buttons to the bottom though: flexbox. So experiment.. I guess. And see what you prefer.

    I'm also not sure if I'm using Tw right. The demos I've watched usually only use the default classes. And that's the approach I tried to stick with in my recent challenge. At the moment, I still prefer writing my own and maybe 20% or less Tailwind. Yes, you're missing something. As I've mentioned earlier, using flexbox to push the buttons down. The flex utilities available are very handy. Also use purge or JIT to make your final CSS output smaller.

  • Grace has commented on Luka's "HTML and CSS ( Flexbox)" solution

    0

    Hi Luka

    This looks fantastic! Just a few improvements needed on the html

    Those buttons should be anchor tags as they would trigger navigation, not an action

    It's up to you if you think those icons are meaningful, in which case you should include a value in the alt attributes. If you think they are not meaningful/important content, leave the alt empty like alt="". Or if you think they are valuable content, the alt text needs to be written slightly differently to what you have now. You don't usually say words like image or picture because they are announced as images anyway, but the word icon -could be helpful. If words are initials or abbreviations like SUV they need to be capitalised for screen readers to read them correctly.

    Really nice looking solution. I hope this extra info is helpful for you in this or other challenges

  • Grace has commented on Gabriel Marques's "HTML and CSS solution using CSS Grid" solution

    0

    Hi

    I'm afraid this doesn't look right at the moment on mobile or any screen sizes.

    The problems are because

    • you're desktop layout starts way too late at 1200+ px
    • you need to remove all the large pixel margins. Use small paddings and margins just to create a little space, not to create the whole layout
    • center content using grid/flex not margins
    • font sizes should never be in px, use a responsive unit like rem
    • use the colors from the style guide, including page background
    • besides the grid, all you really need for this is some max-widths. Test all sorts of screen sizes, not just your full screen (which I expect must be huge)

    Also definitely fix those items highlighted in the report.

    Hopefully that helps and should get this challenge finished off. Good luck

  • Raymart Pamplona has commented on Lakshya Bhatia's "HTML and CSS using Flexbox" solution

    0

    Hey, great work on this one. The layout in desktop and mobile is both good. When resizing, I like how you wrap the card in the bottom, but those border-radius sticks out right.

    The only thing that I would greatly suggest is that, removing the height: 100% in your body tag. Because this limits the body's height, only to 100% of its parent, hence, we are referring now to the viewport or the screen. So when looking in dev tools, your layout is don't have any spacing that prevents them from touching the ceiling and flooring. So removing that will be awesome, and while you're at it, add a padding to the top and bottom of the body tag.

    You did a good job on this one^^

  • Raymart Pamplona has commented on Nayla Gomes's "Profile card component" solution

    0

    Hey, good work on this one. Layout in desktop seems fine and the mobile state is fine as well at some sort of range.

    Regarding your query, I think you did fine using the breakpoint for the background-images, but I suggest that you make it more early than 375, but that is my personal preference.

    Upon looking at dev tools, from the bottom. Your element gets squished in the middle from vertical axis. I saw that you used height: 100vh in the body tag, I recommend not using that, because if you have a large layout, it will cap the body tag which is not good. Instead you can just replace it with min-height: 100vh. Also adding padding to the top and bottom will be awesome.

    Lastly, maybe removing the position: fixed in your attribution will be really good so it won't stick in the screen, if opening up the dev tools.

    Overall, you did a good job on this one^

  • Raymart Pamplona has commented on Adam's "3 Column Card using Tailwind" solution

    0

    Hey, good work on this one. Though I can't really give any advice about tailwind since I haven't used that one. But for you query, we can fix that.

    I see that in your hover state, you are adding border-width: 2px, that's what makes the container's size resize. So instead what you want to do is that.

    Add a declaration in your selector border: 2px solid transparent and when you hover on it, you just transition border-color. Because in your previous hover state, you are adding 2px which will create an extra space right. So to prevent that, you initially add that border-width, so when you hover, you are just adding a color to that border, hence not creating a space and will not make the container resize.

    Also, it would be better if you not used height: 100vh in your body selector, because this will only allow the body tag to have a height of 100% of the viewport. Instead you should add min-height: 100vh. This will make sure that your layout will have a minimum height, based on the viewports height. Also add a padding to the top, as well as the bottom.

    For the button alignment, since the text are being wrapped right, it pushes down the button as well, that is why it changes. I think you could get away with this using grid :>

    Overall, you did good and the mobile state is fine as well^^

  • Raymart Pamplona has commented on Samuel Fuchs's "HTML & CSS only" solution

    0

    Hey, great work on your first challenge here in FEM and good luck for every upcoming challenges that you will take in here!

    For the layout, well it is smaller than the original but it is fine for now. Mobile state is good but at point 554px going down to 375px, the viewport or the screen, hides now your element and therefore, creating a scrollbar at the bottom.

    I would suggest that you make the breakpoint on those ranges, so that you can avoid the appearance of that scrollbar.

    Aside from that, you did a good job considering it is your first challenge^^

  • Agata Liberska has commented on Lpz's "interactive Princing Component" solution

    0

    Hi @Zepolime! I think your solution looks really good (and it works!). The only thing is missing focus outline - it's not a good practice to remove focus styles without providing an alternative.

    I also feel like splitting the price text into multiple spans may be an issue for screen readers, but I'm not totally sure - an interesting thing to look into :)

  • Agata Liberska has commented on Tejas's "Single price grid component" solution

    0

    Hi @Tejas-117, well done on this solution, it looks great! The only issue I found is the missing outline on button focus - it's a bad practice to remove the default focus styles without providing an alternative, custom style - a lot of people rely on them when navigating the page :)

  • William Spanfelner has commented on Antoine KURKA's "HTML/CSS" solution

    1

    Great work! Check out the above report for some quick fixes. The h2 heading, as I understand it should only follow an h1, so there might be a better choice of element there. I found this HTML reference link really useful: https://www.w3schools.com/tags/default.asp. Also, it might be more conventional to keep your css folder on the same level as the assets folder rather than inside it - I kind of see how css might be thought of as an asset, but it should be separate IMHO. ;-) I hope this helps you along your coding journey.

Slack logo

Join our Slack community

Join over 45,000 people taking the challenges, talking about their code, helping each other, and chatting about all things front-end!