Nic

@nicm42
England
575Points

I like problem solving and making things pretty. I also like red.

All Comments

    • HTML
    • CSS

    Four Cards Front-end Webpage using HTML & CSS

    1
    Nic575 | Posted 26 days agocommented on Arjun's "Four card feature section" solution

    This looks like the design to me! I like how you've added comments to your CSS so it's clear what's going on.

    You can fix the three accessibility warnings in the report quite easily.

    The first one is complaining that you have h1 and h3, but no h2. You have to go in order, so you can't have an h3 until you've got an h2 and you can't have an h2 until you've had an h1. So if you change your h3s to h2s, then it'll be happy. The learn more link in the report explains more about this and why it's good to do.

    The other two are effectively complaining that you don't have a <main> tag. The easiest way to fix this is to change your div with class of 'all' to <main class="container all">. You might then want to change your section with an ID of main to a different ID to avoid confusion, but the accessibility report won't care (I think).

    1
    • HTML
    • CSS

    Order summary Component using HTML and CSS

    15
    Nic575 | Posted 26 days agocommented on Tinker's "Order summary component" solution

    This looks great. Your class naming and CSS organisation are good, so I could tell which each bit was referring to in the CSS without having to look at the HTML. I really like the focus states on the buttons.

    In your GitHub readme, the screenshot isn't showing. At the moment you have:

    (./Images/screencapture.png)
    

    What you want is something like:

    ![Desktop](./images/screencapture.png)
    

    (with a small i on images, rather than Images). The bit in square brackets is the alt text.

    1
    • HTML
    • CSS
    • JS

    'Blogr' Landing Page using CSS & tiny JavaScript

    3
    Nic575 | Posted 26 days agocommented on Dragosh Gheceanu's "Blogr landing page" solution

    I like your description of macaroni code :)

    I opened this, wondered why it had so much white space and then realised it's because all your images are missing. You need to upload those to GitHub, as it currently can't find them.

    Your Ubuntu font isn't showing up. It took me a while to spot it, but you've got an extra "-font" when you tell it the custom property to use: --ubuntu-font: "Ubuntu", sans-serif; font-family: var(--ubuntu-font-font);

    Some of your CSS is repeated. So for example, main-para-one and main-para-two have exactly the same code. What you can do is to give both those paragraphs a class of main-para, so then you'd have this:

    .main-para {
      font-size: 1.5rem;
      max-width: 48rem;
      color: hsl(207, 13%, 34%);
      margin-top: 3rem;
    }
    

    What I've found useful, to make the CSS code easier to read, is to space it out a bit more - so leave a blank line beneath each closing curly bracket (like you have after root). And then label each section so it's easy to see on skimming eg

    /**************** Header ****************/
    .header {
      /* Some styles go here */
    }
    
    .logo {
      /* Some more styles go here */
    }
    

    I don't know if you did it on purpose, but only your first dropdown menu works - because the JS is looking at the ID on the first one. If it looks at the class instead, then you can do all three. But that is a little more complicated, as the selector will get all of those lis and you have to loop through them to add/remove the active class when it's clicked.

    2
    • HTML
    • CSS
    • JS

    Time Tracking Dashboard (CSS Grid + Responsive)

    2
    Nic575 | Posted 26 days agocommented on Dainaa's "Time tracking dashboard" solution

    I opened this and saw the same ellipses as in the screenshot - I'm using Firefox. If I open the site in Chrome it looks fine. I think for some reason the browsers are interpreting the space differently, so Firefox is making the ellipses massive. Since Chrome told me they were taking up 21px width, I added max-width 21px to the ellipses in Firefox and then they looked fine.

    Edit: meant to say it looks good and the grid components look fine to me.

    1
    • HTML
    • CSS
    • JS

    Structured with Flex box!

    1
    Nic575 | Posted 3 months agocommented on SAMING's "Ping single column coming soon page" solution

    The trouble with the social media icons is that they're looking for a location on your computer:

    /node_modules/@fortawesome/fontawesome-free/js/brands.js
    

    node-modules isn't a location on your site. What you can do is to download the icons you need, put them in a folder - the images folder would do. And then refer to them in that location.

    0
    • HTML
    • CSS

    3-column preview card component solution

    2
    Nic575 | Posted 3 months agocommented on Megat's "3-column preview card component" solution

    This looks great. Your HTML and CSS are really clean and easy to read.

    The easiest way to get the elements side-by-side are by using Flexbox or Grid. They are hard to understand while you're learning them, but once you've got the hang of them, you'll find them so much easier than floats (or at least, I did). I recommend Wes Bos (https://wesbos.com/) - he has free Flexbox and Grid courses.

    You can also fix some of the accessibility issues. These can be really hard to get your head around. Pretty much all of your issues are complaining about the same thing.

    The <header> tag is only used once for your page header. Take the Frontend Mentor website for an example, the header would be the bar at the top with the logo and links.

    It then wants one heading tag per section. Those are h1, h2, h3, h4, h5, h6. If you were to change your divs for each column to a <section> tag, then change the <heading> tag to an <h1>, that should solve a lot of the accessibility issues FEM is complaining about.

    Talking about headings and headers is really confusing and I can understand why you thought the <header> tag was the right one to use for the header for each column. Reading more about semantic HTML should help you, but it's one of those things that can be a little confusing.

    1
    • HTML
    • CSS

    Stats preview card using HTML & CSS

    7
    Nic575 | Posted 3 months agocommented on Alif Panglima Nurda's "Stats preview card component" solution

    This looks great.

    I have a suggestion with the position: absolute on the body. Absolutely positioning something takes it out of the flow of the document. When it is essentially the document, it's a bit of a weird idea.

    What you can do is instead change it to position: relative. On its own, adding position: relative to an element effectively does nothing, but what it does mean is that any children within it that are absolutely positioned are then positioned relative to their relatively positioned parent, rather than the document.

    Once you do that, you also no longer need the left: 0 and right: 0 on the body.

    1
    • HTML
    • CSS

    Stats preview card with HTML and CSS

    6
    Nic575 | Posted 3 months agocommented on Fer's "Stats preview card component" solution

    Well done for completing this! I can see you've put a lot of thought into it.

    CSS can just be lengthy sometimes, but there are some things you can do to help.

    1. You don't need two media queries. If you took off your min-width: 650px one so all the styles currently in there are just general styles, then any styles that don't fit the max-width: 600px one will be shown. So if your browser width is 601px you'll see the general styles at the top, if your browser width is 600px, you'll see the mobile-specific styles at the bottom. This also solves the problem of not having any styles if your browser is between 601 and 649px.

    Basically, a media query will override any styles not specified above (and outside of) it. So if you have a background colour of red, then you have a max-width: 600px media query where you don't mention the background colour, it will be red no matter what size your browser is. But if you made the background colour green inside the max-width: 600px media query, then above 600px it will be red, below 600px it will be green.

    In your CSS your .attribution has a lot of the same styles regardless of whether it's on desktop or mobile. If you put those styles that are the same at the top, as general styles, then in your mobile media query, you just have to specify the styles that are different. That'll reduce the line count by quite a bit.

    1. You've also made your life a lot harder by repeating the HTML. You just need it once and then you can control how it looks on desktop/mobile with your CSS. So for example, .back-mobile includes all the styles that .back has, plus some extra ones, and the HTML the same, aside from the class name. So if you just have .back, then in your mobile media query, you add those extra styles to .back.

    I hope that makes sense - let me know if it doesn't and I will clarify/give more examples. It might help you to look at other people's solutions for this and see if you can understand how they did it - you can even download everything from their GitHub and change things to see how it all works.

    1
    • HTML
    • CSS
    • JS

    Mobile first using HTML, CSS, JS

    2
    Nic575 | Posted 6 months agocommented on Charissa Ramirez's "Intro component with sign-up form" solution

    This looks great.

    There are debates on the internet about whether you should code mobile-first or desktop-first. Different people find different ways round easier to understand or mean writing less code. So you can do whatever approach you feel works for the thing you're coding.

    I looked at your background-image problem for a while trying to work out what's different between the mobile and desktop ones. Once I spotted it, it's obvious! The mobile one you have quotes around the url, which isn't on the mobile one, but that makes no difference. What does make a difference is that the desktop one has / at the start of the link, so it's trying to find it in https://chawissa.github.io/images/bg-intro-desktop.png - it's missing the signup-form folder in the link. If you get rid of the / the background image will show up.

    1
    • HTML
    • CSS

    ReactJS, Styled-Components

    4
    Nic575 | Posted 6 months agocommented on omisc's "Social proof section" solution

    I looked at this because I'm just learning Styled Components and I found this really easy to read.

    0
    • HTML
    • CSS
    • JS

    Blogr-Landing-Page | Reponsive | HTML,CSS,JS

    4
    Nic575 | Posted 6 months agocommented on Tejas's "Blogr landing page" solution

    Looking at this, I'm disappointed your class names aren't weirder - I thought they were quite logical!

    But I'm finding your CSS a bit odd - you have some mobile specific bits, some desktop specific bits and some tablet specific bits. And some navbar links changes only for specific sizes. It's generally easier to design for one size (either small or big) and then work up or down. That way you don't need so many media queries, and you also don't have to have ones that include min and max, as the bottom one takes precedence.

    1
    • HTML
    • CSS
    • JS

    Blogr

    5
    Nic575 | Posted 7 months agocommented on Sam's "Blogr landing page" solution

    In your app.js you're looking at window resize to show and hide the menu button, but you can just do that in your CSS - so your hamburger menu will have display none on desktop but not in mobile.

    You're also making life hard for yourself by having styles for mobile and styles for desktop because you're then writing some of the same things twice. What you can do is to write all your styles for mobile or desktop, then use media queries to overwrite anything that changes for desktop/mobile. You can even write a mixin for that! If you haven't discovered Kevin Powell on YouTube yet, then he has some good Sass content

    1
    • HTML
    • CSS

    Mobile First CSS HTML CSS GRID FLEXBOX

    2
    Nic575 | Posted 7 months agocommented on Ala Abd Elrahman's "Huddle landing page with alternating feature blocks" solution

    It looks good to me - I scrolled through your CSS and I could tell what each bit was from the class names.

    0
    • HTML
    • CSS
    • JS

    Solution using Flexbox

    7
    Nic575 | Posted 7 months agocommented on Carmen Lopez's "Pod request access landing page" solution

    It looks great, and I like the way you've labelled your CSS for mobile, tablet and desktop.

    I'm not sure why you have the 15% width on the podcast icons, but it's making them look squished and they look much better if you take it off.

    0
    • HTML
    • CSS

    Flexbox, max-width and, pseudo classes for clean code

    2
    Nic575 | Posted 7 months agocommented on Krista Calleja's "Profile card component" solution

    The div around your name and age currently has justify-content space-evenly. So it's evenly spacing them within that div. So you could make it narrower and it would move them both in. But it would be easier to set your justify-content to be center. That puts them both in the centre, right next to each other, so then you'd need a margin on the right of the name or left of the age to move them a bit further apart.

    0
    • HTML
    • CSS
    • JS

    First use of js in project

    1
    Nic575 | Posted 7 months agocommented on Ayoub1Dev's "FAQ accordion card" solution

    This all looks great.

    There are different ways of doing loops, but there's nothing wrong with the for loop. And not all the others will work with querySelectorAll, because JS is fun like that. MDN's page about querySelectorAll gives an example of forEach (which is a loop but doesn't look like it) so if you wanted you could go down a rabbit hole looking that up and trying to understand it.

    There's a couple of other ways to make an accordion. One is to make each of them a checkbox, use CSS to hide the checkbox, and use JS to show the answers if the checkbox is checked. There are also the HTML tags details and summary, which will make an accordion without JS (but that's less useful if you want to practise your JS!)

    0
    • HTML
    • CSS
    • JS

    FAQ accordion card using css grid

    2
    Nic575 | Posted 11 months agocommented on Saheed Shittu's "FAQ accordion card" solution

    I love the way you have the questions font size growing as it opens.

    One thing you could do is to think about at what point you want to change to the desktop view. Based on the design we know it should be the desktop view at 1440px, but is there space to switch to that view at a smaller width? For example, think about someone looking at this page on a tablet, would they like to see the mobile view or desktop view?

    There isn't a right answer, to this, by the way, it's quite subjective. So you might look at it and think it's fine as it is and that's ok.

    0
    • HTML
    • CSS

    Single price grid component using flexbox

    1
    Nic575 | Posted over 1 year agocommented on Bryan Daniswara's "Single price grid component" solution

    This looks really great and I like the effect when hovering over the button.

    The only thing I'd say is that you've overcomplicated your CSS selectors. For example, you only have one div with the class subscription, so in your CSS you just need: .subscription { rather than .single-price .subscription { It just makes it all shorter, easier to read, and avoid typos that will drive you crazy wondering why it's not working.

    You can also use a class twice and only mention it once. So you have a class of title. Rather than having: .single-price .subscription > .title, .single-price .description > .title { you just need .title {

    1
    • HTML
    • CSS

    Single price grid component

    9
    Nic575 | Posted over 1 year agocommented on Dulce's "Single price grid component" solution

    This looks great, I like it.

    Only small thing is that setting width of 350px on mobiles will be a problem for anyone with a mobile smaller than that (the smallest iPhones are 320px).

    0
    • HTML
    • CSS

    Single price grid component

    1
    Nic575 | Posted over 1 year agocommented on Mohammed Boudad's "Single price grid component" solution

    This looks great and your CSS is beautifully easy to read.

    1
    • HTML
    • CSS

    Single price grid component with HTML and CSS.

    1
    Nic575 | Posted over 1 year agocommented on Everton Soares Borges's "Single price grid component" solution

    This looks really good and works really well on different size.

    But you have media queries for devices 1040px and above, and for devices 1030px or less. I could be missing something really obvious about your design, but generally you don't need both.

    All the CSS you have is used at every size, unless you have a media query saying do something different at this size.

    So, for example, if you took everything inside your min-width 1040px media query outside of the media query then your container will be 44.75% wide by default. Since you have a media query for 1039px or less, then the container's width will only change if the browser width is 1039px or less.

    2