Skip to content
OverviewSolutions
9
Comments
251
Roman Filenko
@rfilenko

All comments

  • Roman Filenko•3,335
    @rfilenkoPosted about 4 years ago

    Hey, good job here, but I see a lot of small issues. To debug some of them use rule - * {outline: 1px solid orange;}. This will help to fix overflow problem. A few points for future projects:

    • don't set defined heigth on containers;
    • preferable don't use id's for styling;
    • img { max-width: 100%;} will make all images responsive;
    • it's better to set max-width on wrapping containers instead of just width.
    • use flexbox or css gris for layout, don't use position;
    • for spacing inside use padding.

    Try to fix some issues first, good luck😉

    Cheers, Roman

    2
  • Tiass•295
    @TiasstiassSubmitted about 4 years ago

    Any idea how to use gap for Safari? I just realized it does not appear on Safari, any work-around? Every feedback appreciated

    Responsive data storage component

    2
    Roman Filenko•3,335
    @rfilenkoPosted about 4 years ago

    Hey, you can use grid instead😉, cos currentrly gap for flexbox isn't supported in Safari yet. Or use negative margin/padding for placing items inside flex container.

    Cheers, Roman

    0
  • Roman Filenko•3,335
    @rfilenkoPosted about 4 years ago

    Hey, try to fix console error - Mixed Content: The page at 'https://react-clockapp.vercel.app/' was loaded over HTTPS, but requested an insecure resource 'http://worldtimeapi.org/api/ip'. This request has been blocked; the content must be served over HTTPS. Probably this is the issue here

    1
  • Roman Filenko•3,335
    @rfilenkoPosted about 4 years ago

    Hey, great work. Looks nice, but I have a few notes:

    • semantic html can be a little better
    • try to avoid deep nesting in css, like body .page .container .card-body h2, it's hard to overwrite and maintain styles.

    Cheers, Roman

    1
  • Monica•100
    @monicamclaughlanSubmitted about 4 years ago

    Am in my first few weeks of learning, but wanted to get responses on possibly a simpler way of formatting this without using quite so many classes. Any input appreciated.

    Used lots of margins and padding

    2
    Roman Filenko•3,335
    @rfilenkoPosted about 4 years ago

    Hey Monica, nice try, but there some issues you can easily fix. First, add your your pattern images as background-image with css, this will fix positioning and overflow issues. And work on previous feedback.

    Cheers, Roman

    0
  • Diem Thuy Huynh•10
    @tmeidSubmitted about 4 years ago

    On the line with stars, I used span containing img star, I realized that img star not in span. I tried so hard to fix but I can't. Hope to receive your feedback.

    Desktop-first using flexbox

    2
    Roman Filenko•3,335
    @rfilenkoPosted about 4 years ago

    Hey, good job. Have a few tips for you:

    • .intro > ul > li > span > span - don't use some many selectors to target elements in css, usually one class is enough;
    • not sure what the issue is with the stars - would probably just use imgs or spans in a list (img as background).
    • you can try mobile-first approach on next project.

    Also you can look into sass or css variables. 😉 Cheers, Roman

    1
  • Roman Filenko•3,335
    @rfilenkoPosted about 4 years ago

    Hey Miguel, great work on this one, clean, nice code and responsive as well. Found few smaller issues:

    • add some transition for menu links(hover effect, add focus states as well);
    • hiding overlay by default, adding transition to it on a show;
    • slider buttons can be buttons;
    • grid properties can be written with shorthand like grid-template-column: 3/4 or using grid area to make it simpler.

    Hope this was helpful. Have a good one, Roman

    0
  • Nicolas•20
    @Nicolas-Rodriguez-ChSubmitted about 4 years ago

    I had many issues with adapting the design to desktop screens, I didn't really know what was happening and why it wasn't working.

    I used a lot of flexbox in this project.

    Landing on small screens w/o frameworks

    2
    Roman Filenko•3,335
    @rfilenkoPosted about 4 years ago

    Hey Nocolas, nice work. Some other suggestions how to improve:

    • wriite more semantic html tags (footer should be footer tag, a list of icons - also list of links, etc);
    • add desktop version using mediaqueries;
    • hover on burger menu isn't a proper way to show a menu - make it on click, just few lines in js;
    • in .insurance__text section - with z-index: -5 you can't really click a link;

    Keep practicing, Roman

    0
  • RomainDesson•40
    @RomainDessonSubmitted about 4 years ago

    Hello, thanks to take a look at my solution ! I'm here to take some tips about my code. I'm not really satisfied about that, it works well with the window width asked with the project but when you go on larger width, one div move out from her initial position. I had the impression of design the page only as i see it and i didn't took care about the window width.

    Thanks in advance, i'm working hard to improve myself !

    HTML/CSS training

    1
    Roman Filenko•3,335
    @rfilenkoPosted about 4 years ago

    Hey Romain, good work, but your solution need some improvements. First, I suggest to not position .between block absolutely - you can define negative margin to push it down and add some z-index to make on top of the footer. You can check my solution - I've made similar, a dark challenge and apply some of my techniques from there (grid, hover effects, html structure). Good luck with coding😉

    Cheers, Roman

    0
  • Grace•31,330
    @grace-snowSubmitted about 4 years ago

    This is designed to be a guide for one way to do this challenge mobile-first, using scss variables and some more advanced css techniques like nth-child.

    I used a VS Code extension to compile the scss.

    Note: In a real project I would work with the designer to fix the accessibility issues or color contrast and font sizes.

    Any questions or suggestions are welcome.

    Mobile-first CSS grid layout, with scss, and flexbox in card headers

    8
    Roman Filenko•3,335
    @rfilenkoPosted about 4 years ago

    Hi Grace, congrats, clean and nice code, almost perfect to me 😉

    Cheers, Roman

    2
  • Artem Ponomarenko•365
    @ArtemPonomarenkoSubmitted about 4 years ago

    Hello Mentors,

    It seems like I really straggle with positioning elements. My code is full of hard-coded sizes, widths and heights. Mostly units are px's and %'s . Moreover all the numbers aren't even whole numbers e.g "margin-left: 214.756 px" , "padding-top: -49.589%" etc....
    At the end it's close to the .sketch designs at required screen sizes, but somewhere between.... it's a complete disaster. Nothing is fluid or responsive.

    Trying to use min, max widths but it makes the matter even worse.

    Please advise! Help needed

    Testimonials-slider. Spaghetti code and all sorts of disaster

    2
    Roman Filenko•3,335
    @rfilenkoPosted about 4 years ago

    Hi Artem, you did a great job, mobile looks ok, just a few edits will help with issues you encoutered. You're right about hard-coded values and funky ones like padding-top: -49.589%" - try to avoid them, so my suggestions:

    • first, set box-sizing: border-box for all elements with * selector;
    • don't use fixed values, on containers, for example set max-width, like 1200px;
    • for images it's ok to set width and height, additionally object-fit will help to preserve proper picture proportions.

    Usually, it's better to start with mobile-first approach and then add needed changes with wider viewport width.

    Hope this was helpful😉. Good luck with edits.

    Cheers, Roman

    2