Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Log in with GitHub
OverviewSolutions5Comments32

All comments

  • Joran Minjon• 610

    @DrKlonk

    Posted

    Hi Luka, nice to see you on the platform!

    Some (unordered) things that stick out to me:

    The centering doesn't really work on smaller screens, as Simone pointed out. Use margin auto, flexbox or grid for this to keep it nice and responsive.

    It seems like the responsive.css is the same as style.css. No need to repeat everything in a media query, just the things that are changing.

    The border radius of the card seems a bit off, I think it can be just one value.

    Using rem instead of px is usually preferred, because it increases responsiveness and thereby accessibility.

    Naming classes is hard, but can definitely be improved here. Try to come up with something that describes the content of the element. "Annual-plan" is a decent name in that sense, "div-button" is not. You might want to call that "button-group". Also "rand-text" suggests the text to be random, which it is not.

    Naming things is super important, even more so when collaborating with others.

    The annual-plan class can be centered more cleanly by removing max-width and left properties and introducing margin: 0 2rem or something similar

    Marked as helpful

    0
  • Maja Szczechowicz• 10

    @MSorJinxi

    Submitted

    This is my first project and I would need a feedback because I don't even know what to ask. I'm learning how to code by myself from 2 months.

    What mistakes did I made? What could I have done better? What should I be careful about in such design? How would you rate my code from 1 to 10? ;)

    Thank you for your help and support :D

    Simple HTML and CSS

    3

    Joran Minjon• 610

    @DrKlonk

    Posted

    Hi Maja,

    I agree with the others that CSS Flexbox is a great way to accomplish flexible layouts. I'd like to add Flexbox Froggy as an additional resource to practice with it!

    Happy coding!

    Cheers, Joran

    0
  • Joran Minjon• 610

    @DrKlonk

    Posted

    Hi Agata,

    Nice solution! Good to read (in your readme) that using a framework would do nicely here, especially with the reusable components. I fully agree! It is well worth picking up one of them.

    I especially like the visually hidden headers on sections for our visually impaired visitors.

    Well done and happy coding! Cheers, Joran

    1
  • Jeremy Ng• 200

    @lanechanger

    Submitted

    4/5/2021 update:

    • 167160d 2021-04-06 | fix: removed the flex shrink on rating__stars to prevent the stars from wrapping (HEAD -> master, origin/master)
    • 056d2a1 2021-04-06 | fix: changed justify-content from end to flex-end for better compatibility (chrome)
    • 7ce075f 2021-04-06 | fix: restored margin-left to ratings layout
    • b738a17 2021-04-05 | fix: was using backquote instead of blockquote

    4/4/2021: This was a fun one where I made liberal use of flex containers as a way of dynamically spacing out the components.

    Like the ratings section for example. Initially I was practicing a for loop on it and made it such that margin-left = 16px * (x - 1) where x is each rating component. So the first one would have margin-left of 0, the 2nd would have 16px, the 3rd would have 32px.

    Which works better for future proofing if additional components were to be added. But I decided to make them flex and assume that only 3 items are used, so the top one got justify-content of start, the 2nd one center, and the 3rd one end with the extra space being shrunk dynamically as the viewport shrunk. And I really liked how that turned out along with how the other flex containers shrink. It was mostly done through flex: 0 1 <basis> which prevented them from growing, but allowed them to shrink once the viewport got smaller than their basis.

    Dynamic spacing using nested flex layouts

    2

    Joran Minjon• 610

    @DrKlonk

    Posted

    Hi Jeremy,

    Nice job overall! Some things I've noticed:

    The third rating box should be: justify-content: flex-end (instead of end).

    The stars wrap from 640-653 pixels wide for the middle rating, that looks a bit weird. Also, from 792 to 426 pixels wide the stars seem to move upon resizing. They don't align properly with the other ratings either.

    All this is because the rating__stars resizes because of the flex-shrink, but the stars have a fixed width. I think it is better to remove the flex-shrink from this class.

    Btw, flex: 0 1 auto is the default, so you don't even have to set the growth and shrink explicitly!

    Cheers, Joran

    1
  • Raymart Pamplona• 16,140

    @pikapikamart

    Submitted

    This challenge was really fun. My first draft was full of animation but I couldn't pull that one formula in terms of checking boundary so I had to remove it. Limited myself to using only few html elements. So I need to be very careful in my js since i'm just reusing those elements.

    Anyway, have a look and drop your comments in my work^^. I will also create the spock version

    13th challenge with pure JS and SCSS^

    3

    Joran Minjon• 610

    @DrKlonk

    Posted

    Hi Raymart!

    Reaaaaaally well done on this one. Looks good, works good. A nice example for everyone to follow.

    Particularly, I like the little animation on the 'rules' text and the fact that you don't get points reducted when you lose.

    A minor thing I found in the scss: I think technically the flex classes like flex--j-between are utility classes and not variables. I'd expect a _variables.scss to contain only scss variables. But maybe that's just me. You could check out the 7-1 pattern to structure SCSS. I really like it.

    Just one minor thing I found in the code: in displayResult() the winner always gets set to playerChoice, although I don't think it affects anything.

    Again, very well done!

    Cheers, Joran

    1
  • wisniewskiz• 55

    @wisniewskiz

    Submitted

    I used parcel bundler to compile all of my sass into a distribution ready file.

    I'm not sure if I went about the cleanest or easiest way to create the layout using grid and flexbox, but the end results are useable. If you see anyway that I could have wrote more concise code please let me know!

    This is my first solution to submit and I am very exited to learn and grow with everybody here.

    Pod request responsive landing page using css grid and flexbox

    1

    Joran Minjon• 610

    @DrKlonk

    Posted

    Hi Zach!

    Nice job for your first challenge! Don't be misled by the word newbie for this one, it is quite tricky. The desktop version looks nice!

    Main thing: responsiveness I see some problems when I resize the screen to a mobile width, it kind of breaks. It works at 376px and lower, but everything in between is a bit wonky. If I resize the window, I get horizontal scrolling. And if I resize within devtools, at 775px for instance, the paragraph text size is too small because of the automatic resizing. On tablet sizes, the background image get pushed to the left, eventually going past the left side of hero__cta and the screen.

    I think these issues are caused by the grid that can't fit the screen.

    Random other things

    • You can add some line-height to the paragraph to gice it some space between lines.
    • There are no error messages for the email input.
    • It would also be nice if the google podcasts logo was centered vertically. Nothing a little flexbox can't fix.

    If you like, you can check out my solution for this challenge as well, it can at least help if you want to make the error messages.

    Don't be discouraged by my comments, you already took a very good step to join the community. Just trying to help!

    Keep on coding! You'll just get better and better.

    Cheers, Joran

    1
  • Saurabh Ramesh Kacholiya• 50

    @saurabhkacholiya

    Submitted

    1. Can anyone look into the code and see if my structuring of code is up to the standard for HTML and CSS?
    2. Can you please rate my design skill and point out areas of improvement?
    3. Is there any better way than the code has been written any blog or post to follow to improve CSS?

    Mobile first approach,flexbox

    1

    Joran Minjon• 610

    @DrKlonk

    Posted

    Hi Saurabh,

    Decent work! The site resizes properly and I like that there is some feedback on clicking buttons (even though it is not that meaningful).

    Some things to consider: Meta:

    • Please use a separate file for your css. It is bad practice to put everything in one file. You can import the css file in your <head>.
    • The class names are not consistent enough. Sometimes you do use the elements name (section/span/etc), sometimes you don't. Try to make the class names represent what the CSS is about. If the CSS is specific for the footer, call it 'footer'. If it is more of a utility class that gets reused, call it 'info-text-left' or something like that. Reusing a class named 'grow-together' for the same thing is a bit weird.
    • Do fix the html and accessibility errors generated by Frontend Mentor
    • I always like a cursor: pointer on buttons.

    Design:

    • The text on nearly all buttons is quite tiny. Go for at least 14px (or 0.875rem).
    • The top section needs more breathing room. It is quite different from the design in font size and margin of the text.
    • The big image with the colored dots looks stretched in the vertical direction. You can just remove the height property to fix this.
    • The headers from "Grow together" and such are also too small. Compare it with the design and see the difference.
    • You should hardly ever text-align: center a paragraph of text. It makes it hard to read. Check the design here as well, it is different.
    • The "ready to build" part actually has a bit too much space in my opinion.
    • The wavy image near the bottom has some whitespace on its bottom on some screen widths for me in Chrome. I can't find out why, but it shouldn't be there. It might be because in the HTML the <img> is just floating there in between the main and the footer. You could make it the background of a before pseudo element of the footer, maybe that helps. Removing the width: 100% also removes the whitespace, but the result might not be what you want.
    • In the footer I'd expect the subscribe button to also trigger the alert but it didn't.

    A lot of stuff! It's not all as important, but definitely enough room to improve.

    Happy coding!

    Cheers, Joran

    0
  • Joran Minjon• 610

    @DrKlonk

    Posted

    Hi Vitor,

    I like it! There are some minor things to improve, but overall it works nicely.

    I somehow didn't even know that creating the <input> inside of the <label> element was a thing. Mozilla shows it as an alternative, so it seems like a completely valid option. I asked a question in Slack#best-practices as to what is the preferred method.

    You can also toggle a class on an element in javascript with element.classList.toggle('.className'). I think that would lead to some cleaner code in your solution.

    However, I think I prefer to handle the dark theme switch method described here. That saves adding classes to all items and writing those classes separately in the CSS.

    In the smaller cards all the arrows are green and point upwards. Easy fix!

    All in all, a great job!

    Cheers, Joran

    0
  • Joran Minjon• 610

    @DrKlonk

    Posted

    Hi Vinicius,

    I like it a lot! The responsiveness is on point and it all looks nice.

    Some minor details:

    • On smaller screens, the headings of the cards could use some room to breathe on the top.
    • The paragraphs of text in the cards should be <p> instead of <span>.
    • The classless div inside of the get-started div could be removed. It also lacks padding when resizing (check 346px for instance)
    • The links in the footer should semantically be a list, I think
    • You have some purposefully empty columns in the footer grid, for spacing. I think that is not that nice. Also, on 983px wide, the column for the first set of links is much smaller than for the second. I would not expect that. I think that extra space is for positioning the social icons properly, but that could be done without the empty columns too. You could also position the items inside the social column differently to fix this.
    • The <picture> tag is mainly used for multiple responsive images. I don't think it is needed here.

    Again, it may look like a lot, but they are all quite minor and the page works as it should.

    Cheers, Joran

    1
  • AbdulKareem• 170

    @seniorteck

    Submitted

    after spending a couple of days trying to style the body with background property. I finally figured I was styling it in the wrong way I think, so I created a div with an empty element for the background to make it work, I don't know if this is the best practice, I used HTML & CSS for the project, would have loved to use sass but I am still a beginner.

    any thought on how I can improve better on my code. pls would love to hear it. thank you

    Chat-app illustration Html & Css

    3

    Joran Minjon• 610

    @DrKlonk

    Posted

    Hi AbdulKareem,

    I would also check out the accessibility issues in the generated report. Those are quite relevant.

    Design wise, the biggest thing for me would be the font and the way the text is styled. Check out here how to add a font to your page.

    The padding on the paragraph makes it off line with the heading, which looks strange. I'd just remove the padding.

    Also, your background is a bit darker than the design, making for less contrast on the paragraph, which makes it a bit more difficult to read.

    In the code you use px and em for sizes a lot, I think it's well worth looking into using rems for all your sizes. It allows you to resize basically your entire site by setting the font-size in a media query. Check out this article on px vs rem, or do some googling on it.

    Cheers, Joran

    0
  • Joran Minjon• 610

    @DrKlonk

    Posted

    Looks good overall!

    In the share button I see some room for improvement.

    It would be nice to have it disable if you click outside of it as well.

    It is also a bit weird that on smaller screens it covers the name and date (but maybe that was in the design).

    In the code it is not great to have it dependent on the color of the button. I would probably toggle a class (button.classList.toggle('active')) on the button on click and fix everything in the CSS based on if the button has that class or not.

    You could select the icon-social element in CSS with the "~" selector I think.

    Also, it looks like you are tyring to use BEM for naming classes, but it's not consistent. If title__content is an element of the content block, it should be content__title. After that you also use contact-user and userInfo, which are a bit ambiguous and use a different structure (hyphen and camelCase). Naming classes can definitely be tough, but can lead to much easier to read code. So it's well worth looking into! It's one of my own points of improvement as well.

    Cheers, Joran

    Marked as helpful

    1
  • Marco• 265

    @MarcoCarpo

    Submitted

    Hi everyone, this is one of my first projects using React.js (still learning this). How does it look to you?

    Also, does anyone have any idea how to remove the extra space above and below the text? (e.g. see the buttons, the text inside seems off-center due to these spaces) Is it possible to do this in Sass or do I need to apply changes directly to the font before importing it into the stylesheets? I mean, maybe I could implement a mixin that automatically sets the line-height based on the font-size. Or is this problem solved in another way? This is a issue that I often encounter and I still don't understand how to solve it. In any case, thanks for your attention!

    Job listings with filtering challenge with React.js

    2

    Joran Minjon• 610

    @DrKlonk

    Posted

    Looks good to me!

    What do you mean with the extra space below the text? Which text(s) do you mean?

    I think the buttons look a bit off centre because of 1) the font-family and 2) the fact that these words have little letters going down of the baseline (like g, p, or y for instance), making it a bit more obvious. It's a bit harder to get annoyed with the pixels in "Python" than it is in "Frontend", in my opinion.

    Here's my font-family proof: If you change the font to monospace, the clickable labels look fine.

    Fix for the smaller labels: On the labels like "NEW!" you could just add some more padding to the top in the job__feat class. I don't see much problem with that, except when you decide to change the complete font family, in which case you'd need to adapt at most 2 lines of CSS. No biggy.

    Minor improvement: I'd also add a good old cursor: pointer; to the job__position class.

    Cheers, Joran

    2
  • Joran Minjon• 610

    @DrKlonk

    Posted

    Hi Mayokun, there are some other issues I found (after some url hacking) as well. Most of them easy fixes!

    • I'd remove the transition on the main component, it is moving slowly on resizing. That looks weird.
    • The rules box is the same color as the score box, making it hard to see the border in the top right part. Some box-shadow would do nicely, I think
    • from 376px to ~585px wide, the score card is out of frame, while it is in frame for smaller and bigger sizes. You could just use the same process to make this fit
    • If the screen is super wide, the Rules-button flies away a bit. Try a max-width on the body to keep it close to the game.
    • Don't use a <button> inside an <a> tag, that is semantically incorrect. They can both be used to provide links to stuff. My default is <a> for (external) links, <button> for actions on the same page. So here, an anchor with styling to make it look like a button would suffice.
    • There is still some console.logging done when clicking the buttons
    • I dont like the red text inside the "play again" button. It makes it look like I lose all the time ;)

    The game works fine though! That's the most important. Good job.

    Cheers, Joran

    1
  • Joran Minjon• 610

    @DrKlonk

    Posted

    You should remove the "/picked/scissors" from the live url. Now, this doesn't work out of the box.

    0
  • Sebastian T• 120

    @tssessy

    Submitted

    Had a lot of problems with the gradient linear background .. not easy to do it in css. Still don`t think is the best solution. Any help comments feedback is apreciated.

    responsive css chat app

    3

    Joran Minjon• 610

    @DrKlonk

    Posted

    Hi Sebastian,

    Here are some things I noticed:

    • The right gradient is actually on top of the mobile phone on smaller screens (it is not a background image in that sense)
    • There is a "filler" div which does not seem to do anything
    • There is a gradient div in the left div which should be removed (it is a duplicate which shows on really wide screens)

    A nicer way to fix this, in my opinion, would be to use the before and after pseudo element on the body to position the purple things. Check out the way @grace-snow did it here

    On the use of CSS grid:

    • You are overriding the areas with the columns in the middle div.
    • The columns are a bit strange. You are kind of building a 3 column layout with 12 columns. Why not use it like so? grid-template-columns: "4fr 5fr 3fr"; grid-template-areas: "l m r"; But in the end, I think CSS grid is a bit much.

    I think using a flebox with just the phone and the text elements in it would suffice for this challenge.

    Also, try to use more semantic elements like <main> and give your classes more meaningful names, for instance "introduction" instead of "text".

    Keep on happy coding!

    Cheers, Joran

    0
  • ZAHHARI OTMANE• 200

    @otmanezahhari

    Submitted

    If anybody would like to review my code and give me feedback that would be great!

    Any other type of feedback is appreciated as well! Thank You!

    Pod-request-access-landing-page

    1

    Joran Minjon• 610

    @DrKlonk

    Posted

    Hi Otman,

    I was browsing through the solutions for this challenge and went back a long time to see how others fixed it.

    So this advice is a bit late, but better late than never.

    • The green dots are continously moving. Is that on purpose? I find it quite distracting.
    • You can import the svgs just like you normally would in the <img> tag, in the src attribute. That way, you can keep your html a bit cleaner.
    • You could make the podcast logos a bit nicer by not giving them height and width of 100%, but creating a lexbox and aligning them in the center. Then the google podcasts logo wouldn't look stretched. Because now, they all get the same height. Fiddling around with the margin is not the way to go.
    • The error message is in a different font (Times) and the input field as well (Arial). That is a bit strange, you can just use the same as the text. I think that would be nicer.

    All in all, nice job! The responsiveness works well.

    Cheers, Joran

    0
  • Agata Liberska• 4,015

    @AgataLiberska

    Submitted

    Hi! I really struggled with svg images in this challenge and wasn't able to change the color to what it was in the design - change of fill on svg path didn't work, even when manually changed in the code. I think it's because of the color matrix in the file, but I couldn't find anything helpful in my search - either because I didn't find the right way to describe the problem or because I'm overcomplicating it :)

    Either way, I'd be really grateful for any help on that :)

    Mobile First, Scss, Vanilla Js, simple form validation

    3

    Joran Minjon• 610

    @DrKlonk

    Posted

    Hi Agata,

    I "fixed" the SVG problem in my solution of this challenge. I put two svg filters inside the html so I can reference them in CSS. It is very ugly though and I did not find a way to put them in an external file and make it work. That would be an ideal solution.

    I think you're right that changing the fill does not work when there is an feColorMatrix in play. So you have to change the filter to change the color, or change the SVG altogether and wrestle out the color matrix.

    Cheers, Joran

    1
  • Milos• 330

    @MilosSimic994

    Submitted

    Hello everyone, i yust upgraded this project. I have a question about the progress bar,

    • I solved this with two DIVs. but i don't think thet's the best practices?

    -What do you think about it and how would you do this?

    -which is your method for styling range input?

    Thanks, any feedback is welcome :D

    Fylo data storage using CSS HTML

    1

    Joran Minjon• 610

    @DrKlonk

    Posted

    Hi Milo,

    On a quick glance, this looks like an excellent opportunity to use the <meter> element. I must say I've never used it, haha. I just know of the existence for a short time.

    Cheers, Joran

    1
  • Joran Minjon• 610

    @DrKlonk

    Posted

    Hi Flo,

    Nice job! Everything seems to work fine. Disclaimer in advance: I've never used React. Here are the things I've noticed.

    You said in Slack: "Apologies for slightly going around the requirements but I used React + styled-components." Remember there are no requirements on how to fix the website you are making. You are free to use any tool, framework or language you want!

    The background seems a bit opaque. I didn't find where this happens, but it's just a bit lighter than the design.

    Is there any way to import the svg into CardBackground by not copy pasting the whole thing in there? Now it's just a lot of unreadable svg data, which I think should not be a component of itself. Say, if you want to add something to the background, then this svg element really gets in the way of reading the other code.

    You used object destructuring in the Card component, like so:

    	const { name, age, city } = characterInfo;
        ...
}```

But you can also do it in the parameter directly, like `function Card({name, age, city}, ...` This saves a line of code, but also makes it a slightly bit clearer where the parameters come from.

All in all, great job, and see you around!

Cheers,
Joran
    0
  • Michal• 665

    @mbart13

    Submitted

    I struggled a lot with positioning and responsivness, but I think it turned out fine. If you think something could be improved or have any other suggestions, please let me know

    SASS, Flexbox, JavaScript

    2

    Joran Minjon• 610

    @DrKlonk

    Posted

    Hi Michal,

    It looks great! The responsiveness works fine and the positioning of the elements is where they should be. The code looks clean and maintainable as well!

    The main thing I think is a bit iffy, is the dependance on the transition in the updateSlide method. You now need the transition (that is defined in CSS) to always play, which is caused by adding the class in the line above it. It seems a bit strange to add an event listener in a method like this. I think I'd rather see the effect working, regardless of the transition in CSS.

    For instance, right now, setting the transition time to 0.0s breaks the change. I don't think that should happen. But maybe that's just me.

    All in all, it does work as is and in the end, you did a really good job!

    Cheers, Joran

    1
  • Rayane• 1,935

    @RayaneBengaoui

    Submitted

    Hello everyone ! 🙂

    Here is my version of this challenge, any feedback/remark is appreciated ! I have a few questions:

    • The API is blocked by adblocker such as uBlock, is there a way to avoid it ?

    • I tried to hide the API key with the netlify GUI, but is there a simplest way to do it with Node.js for example ?

    • If the text returned is very long my boxes info-container__box get pushed. How can I make sure that no matter the length of the text I receive, the text fit the box ? Or dynamically adjust the font-size ?

    Have a nice day ☀️

    IP Tracker - HTML/CSS/JS

    2

    Joran Minjon• 610

    @DrKlonk

    Posted

    Usually the way for me to hide an api key would be as an environment variable. You can get those in a Node server by installing the package dotenv, adding a .env file and accessing it in the server by using process.env.VARIABLE_NAME. More on it here.

    Do not include it in git though, if you really want it hidden.

    Btw, I didn't do this challenge yet and have never used Netlify, so I don't know if your way is even better.

    1
  • Nderim Sali• 40

    @nderimsali096

    Submitted

    Responsive landing page with the help of media queries. I have used Html , Css and SCSS only . The app page is built for 1440px Desktop and 375px Mobile. Constructive criticism is very welcomed in every area of the project. Thank you!!

    Responsive landing page using mdedia queries

    2

    Joran Minjon• 610

    @DrKlonk

    Posted

    I agree with Agatha that resizing does not really work properly. I think you are combining flexbox and relative/absolute postitioning a bit liberally. There are also a lot of HTML issues you can fix first.

    The main thing design-wise is the different font. You can easily use fonts by importing them in HTML or CSS. It would make a big improvement with little work. Check out this link for how to do it: https://developers.google.com/fonts/docs/getting_started

    Good luck and have fun!

    Cheers, Joran

    1
  • Joran Minjon• 610

    @DrKlonk

    Submitted

    I am mainly wondering if the way I did the background-images is the way to go.

    I am trying to get better at naming classes, using the right HTML elements and accessibility, so it would be nice to get some pointers on that.

    Any other comments are, of course, very welcome!

    Social proof using Flexbox

    3

    Joran Minjon• 610

    @DrKlonk

    Posted

    Thanks both! I made the updates and changes. Ready for shipping now ;)

    0
  • Joran Minjon• 610

    @DrKlonk

    Posted

    Yes, you can use the pseudo-class "invalid" for that.

    Like so: https://codepen.io/drklonk/pen/xxRavNG

    If you need you can also combine it with the active state (which is also a pseudo-class).

    Cheers! Joran

    0