Request path contains unescaped characters
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Request path contains unescaped characters
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
OverviewSolutions23Comments287

All comments

  • Ricardo Fuentes Urbina• 370

    @RicardoFuentes437

    Submitted

    What do you guys think about the responsiveness? What do you think about the visual aspect of the page? does it look good? What are some practices that you think can be improved?

    Html, Sass, Javascript, Flexbox

    #sass/scss#bootstrap

    1

    Agata Liberska• 4,015

    @AgataLiberska

    Posted

    Hi Ricardo, visually your solution is nice, doesn't overflow on smaller viewports which is probably the most likely issue on cards like this. However, you could definitely simplify your html here, at the moment pretty much every element like a heading or a paragraph is wrapped in a separate div and that's really not necessary. You're also missing the <main> landmark and the card could really be an <article>, that's the element that components like this tend to match the best.

    For your scores, you're using input type="button" which also isn't the best option. Buttons and button type inputs should be used to perform an action, for example favouriting a page, or adding a bookmark. Here, we're selecting one of 5 options - and if you have a number of options and have to only select one, this really calls for radio inputs.

    I would also treat this as a form and use a submit event to update the selected score.

    Hope this helps :)

    Marked as helpful

    0
  • Mario Lisbona• 150

    @MarioLisbona

    Submitted

    I found this project quite easy as i had just completed the 'intro component with sign-up form' with multiple input field validations.

    I wanted to improve it so i create a function with all the logic for the two different error messages for empty input or invalid input. This worked really well so i updated the previous project with a similar function which cleaned up the code.

    One issue i have is that i have used preventDefault() to that the form from being submited while the inputs are invalid. However, the form is still not submitting when all the inputs are valid

    Base Apparel coming soon page with CSS and JS for email validation

    1

    Agata Liberska• 4,015

    @AgataLiberska

    Posted

    Hi Mario, page is looking really nice on desktop but something is off on mobile - on some screens the image looks to be covering up the heading. I think it would be a lot simpler if you didn't try to position it absolutely - have you tried just adding it with an img tag instead?

    The background image also is repeating which I'm not sure is the case in the original design.

    Nice work with form validation - I think you could maybe try to add validation as person starts correcting the email address? could be an extra challenge :) and as for preventDefault, it just prevents form from submitting to the page specified in the action attribute via the method specified in the method attribute. If you use this, you would need to provide an alternative way of handling the submission but don't worry about that for now. You can just set the input value to "" and display a success message to give it some pizzaz :)

    Marked as helpful

    0
  • Agata Liberska• 4,015

    @AgataLiberska

    Posted

    Hi! Looking good and working just fine :) Definitely check out the accessibility report - the <main> landmark is missing, and the button we're using to update needs some type of labelling to be accessible. I'd also advise to add width and a height whenever you add an img - to avoid layout shifts.

    One probably very nitpicky thing - when I reload the card, the button and the quote marks keep moving up - this only lasts a fraction of a second but can look a bit like a glitch. I would suggest setting up a min-height for your advice-txt container so there always is some space there - even if it's just the height of a single line of text (I'd personally go for two lines and center vertically if it's just one line but that's your call). This layout shift wouldn't really be an issue because it happens after a user interaction but I think with some min height it would look more polished.

    Hope this helps :)

    Marked as helpful

    1
  • Agata Liberska• 4,015

    @AgataLiberska

    Posted

    Hi Sergio! Your solution works as it should and visually it's really nice as well, I like the slow transitions and hover states :)

    I would, however, simplify your html and your js (although I have a feeling you also wanted to practice some things which is totally fine!).

    As for the html: at the top of your card you have an image, a heading and then a paragraph - all of them are wrapped in their individual divs which really are not needed here. It would be enough to set the image to display inline-block or block so it sits on its own line.

    Definitely a good call having the ratings as radio inputs, I'm not sure about the styling - I would usually set the inputs to be visually hidden and use labels for selection - this also makes it easier on mobile where you need the target to be slightly bigger than the radio button itself for people with sausage fingers like myself :) you'd just need to have an input come before the label in your markup and then you can use something like input:focus > label or input:selected > label. And here is also how you can simplify your js - no need to toggle classes here, just use the :selected pseudoselector to get the orange background.

    You also can get the submitted rating value in the submit event so there is no need to have a separate function to extract it. Try console.logging the event to figure out how to get there.

    Overall I think you've put quite a lot of effort here and although things could be done simpler, it's great to see you've got some creativity and are really trying to figure out things on your own, well done :)

    Hope this helps, happy coding!

    2
  • Agata Liberska• 4,015

    @AgataLiberska

    Posted

    Hi! Nice work on this solution, visually I think this looks really nice. I would just work on the html - for a simple page like this, you're using quite a lot of nested divs which I personally think are unnecessary here, a lot of those could just be removed with no effect on the page. If you have a few block elements you don't really need to wrap them in a div unless you're grouping them for styling purposes etc. :)

    Nice work using a list for the social icons in the footer (those will need some labels/titles to validate though) but I think the three divs in 'we're different' section also should be an unordered list. And the mobile menu toggle should definitely be a button :)

    Hope this helps !

    Marked as helpful

    0
  • kefiiiiR• 70

    @kefiiiiR

    Submitted

    Eventually, I suppose it works as it should work. I had a bit issue with active state of the buttons which I solved by using another loop inside the first loop (I could not think of another solution).

    Interactive rating, HTML/CSS/JS

    1

    Agata Liberska• 4,015

    @AgataLiberska

    Posted

    Hello :)

    I think you made this a bit more difficult for yourself going for buttons with ratings where this really is a small form with 5 radio inputs - So I would visually hide the inputs themselves (key word is visually , display:none wouldn't be a great choice) and style the labels to be the circles you click and then pass this to the submit event :)

    And if you want to change the look of the buttons after pressing one, loop or forEach is an ok choice, but I would simplify your code by having an 'active' class and then just using classList.toggle('active')

    hope this helps :)

    Marked as helpful

    2
  • Agata Liberska• 4,015

    @AgataLiberska

    Posted

    Hi Zauri! Visually this solution is really great, it's very nicely responsive - but html needs some revision. You're missing a <main> landmark here, and if you use a <section> tag, it will need a heading (maybe use the logo as heading?). also for the progress bar, <meter> tag would be perfect (but I think it may be a bit tricky to style)

    Hope this helps :)

    Marked as helpful

    0
  • Shahid Khan• 20

    @skafridi07

    Submitted

    Although it seems easy, it took a lot of time and effort to specifically set the media query at max-width: 376px to ensure that the card layout behaves responsively. Because of the bem-css naming convention, which I used in this project, it is much simpler to read and debug the code. Despite the time commitment, it was worthwhile. The font-weights, font-families, and colors all have their own custom variables that I first set up. The layout was then constructed using Flexbox, and where necessary, margins and paddings were added. It was an excellent experience all around.

    Responsive card design using flexbox and media query

    #bem

    1

    Agata Liberska• 4,015

    @AgataLiberska

    Posted

    Hi Shahid! Card is looking nice :)

    One thing I would advise is to simply start with the mobile design instead of using max width media queries here. It's simply easier (mobile designs are usually simpler) and allows for fewer lines of css. For example on mobile, the image sits on top of the text container - which you achieve by setting flex-direction:column but if you started from mobile and added display flex at 400px and wider, this wouldn't have ben necessary at all because those elements just sit on top of each other by default.

    Another thing is that 378px is not really enough space for the card to display nicely, so I would move that query up to 400, maybe 450px?

    And lastly, for accessiblity, the image of the perfume definitely needs and alt text and you're also missing a <main> landmark :) and for more semantic html, <article> tag is perfect for cards like this

    Hope this helps :)

    0
  • Agata Liberska• 4,015

    @AgataLiberska

    Posted

    Hi Nader, I think the background in this challenge is particularly tricky! I would have two images added in rather than just the one, set it to no repeat and then position each individually - or just add them in with the html, might be easier to position that way :)

    For the profile image, I would probably move it to your bottom div, so it's right at the top and then move it up using tranform:translateY property :)

    Other than that, I would have another look at what tags youre using here. <article> is a perfect tag to use for card components like this, and the stats should really be a list - and you definitely don't need to put each bit of text in it's separate div. I'd suggest something like this:

    <ul>
<li><span>80K</span> followers</li>

    And if I could give one more bit of advice, try to use class names that are a bit more descriptive. I think "profile" is good, "profile-card" would be better, and then instead of top/bottom something like cover or banner and profile-info? It will make your work a lot easier on bigger challenges! :)

    Hope this helps, happy coding!

    Marked as helpful

    0
  • Agata Liberska• 4,015

    @AgataLiberska

    Posted

    Hi Eslam, I think it would be a good idea to review what html tags you're using here. It looks like you're using heading tags ust based on the styles you can see which isn't the best approach. Think about hierarchy here - it makes sense for the name of the perfume to be the main heading, but all other text could just be inside <p> tags or <span>s.

    I would also rethink your button. You've added hover styles to your green div which look nice but it would suggest to me that I can click this and something would happen - but only the text is an actual link. So Instead of using a div, you could just have an anchor tag and style it to look like a button:

    <a href="/"><img src="..."> Add to Cart</a>

    Make sure to take a look at the report above as well - the image urls should use / rather than \ and make sure to use landmark elements, <main> is definitely missing from here. I would also use <article> tag for the card.

    Also, for the perfume image I think adding a good alt text would also be very important, as this isn't a purely decorative image, it shows a product we're trying to sell.

    I also had a look at your css - I think in a simple project like this there is no need to combine your selectors, as you've only really got one thing with the class price or cart. Personally I think combining selectors really needs to be thought through as in a bigger project you may run into specificity conflicts and end up with a bunch of !importants which just isn't fun to work with :)

    Hope this help, happy coding!

    1
  • Odiesta• 100

    @Odiesta

    Submitted

    I struggle to center the container. Then after a lot of googling i use display flex and align-items center. I was unsure whether to target the html tag in css or tag inside particular css class. Is it best to target html tag inside classes or id because it is more specific?

    QR Code Component with CSS

    2

    Agata Liberska• 4,015

    @AgataLiberska

    Posted

    Hi! I think setting those flex styles on a container class is definitely a good idea. It makes it more reusable and if you were to add something else in the body, it wouldn't be affected. You could also add justify-content:center to your styles there to center horizontally - in your solution this is done with margin:auto which also works fine, but in a bigger project if you wanted to reuse this, I think I would prefer all centering styles to be under one class (rather than a container and the component itself if that makes sense).

    Another way to achieve the same result is to use display:grid;place-items:center; - just a matter of preference which one you go for.

    What I would like to see in your solution is some more semantic tags. so your container could be a <main> and then for the card itself, I would add an <article> tag inside the container. Also don't forget to add a width and height to your image :)

    If we're also looking at accessibility, it might be a good idea to add a link around the QR image :)

    Marked as helpful

    3
  • imad• 3,310

    @imadbg01

    Submitted

    The checkbox was a letter bit tricky for me to understand at first, but I was able to found some blog post in css-Tricks that help me out, please feel free to give your feedback 😊

    Pricing component with toggle solution

    #accessibility

    1

    Agata Liberska• 4,015

    @AgataLiberska

    Posted

    Hi Imad!

    So there needs to be some improvements here when it comes to matching the design - you have a min height set on the cards, which stretches them unnecessarily. In general with components like this there is no need to set the height like this, instead I would control the size of the element with the content and some padding. Other than that I think it looks nice :)

    With the checkbox input though - first of all it really does need to either have a label or at least an aria-label. Also, I think it's working the other way round? The price for monthly shows up as 249, and annually as 24.99 :D no worries, it's an easy mistake to make when you're focusing on how to build something you've never done before!

    Anyway, I'm not sure that a checkbox here is the best option - I know that it is probably the most common design pattern with toggles like this so I don't blame you for choosing it! But you're really choosing between two options here (instead of choosing to have something included or not) - which really calls for radio inputs to be used. A checkbox would be for something optional which you can choose or ignore, which isn't the case here. I've got one or two projects which have a toggle built on two or three radio buttons if you want to take a look, but I am also certain that Grace Snow has done this exact challenge in a beautiful way (I know because I totally used it as a cheat sheet when building my solutions :D) oh here it is

    Anyway hope you find this helpful, happy coding! :D

    Marked as helpful

    1
  • Chris Ekstedt• 80

    @cekstedt

    Submitted

    Are there any best practices, in semantics or styling, that you would suggest as an improvement to my code?

    QR Code Component

    2

    Agata Liberska• 4,015

    @AgataLiberska

    Posted

    Hi Chris! Nice work, your solution looks just like the design, and it's good to see some semantic elements :) One advice I would give is that it's a good idea to get in the habit of including width and height on your img elements to avoid any shifts in layout - that's something that google would pick up on in audits and score you down for. I would also link to google fonts in your <head> element in your stylesheet rather than importing it from css - that way it loads at the same time as the stylesheet - right now it will only download after the browser is done downloading and parsing your stylesheet. Doesn't make much difference when files are tiny but when your code base grows, it can have an effect on loading :)

    Marked as helpful

    0
  • Agata Liberska• 4,015

    @AgataLiberska

    Posted

    Hi Lucas, nice work on styling here - your solution looks great. There are some overflow issues on smaller devices which you may want to look into though, and I'd definitely modify your media query - you've set the width of the card to 800px, but making the switch from mobile to desktop at 700px - so not quite enough space for the card.

    I would also recommend to look at accessibility here - a person using keyboard to navigate your solution wouldn't be able to toggle any of the accordions - the toggle should definitely be a focusable element, like a button.

    I would also like to be able to open more than one accordion at a time :)

    Hope this helps!

    Marked as helpful

    1
  • Agata Liberska• 4,015

    @AgataLiberska

    Posted

    Hi Mohamed, nice work :) Awesome that you remembered to include you landmark elements :)

    However, it would be a good idea for you to really consider why you're applying certain styles. For example, you've set your body to display: flex; but you're not actually taking advantage of this - in fact, it actually messes things up a bit - when I open your solution, the card is stuck to the left of the screen - because of that display: flex property. You've also set a blue background on all elements, which you then had to override on your content container - it would've been a lot simpler to set the blue background on the body and white background on the card. Setting the height property on your card is also quite unnecessary - and again, it does mess things up on some larger mobile viewports where the white card is tiny and all content is overflowing it. Let the content take up the space that it needs, and space it out with some padding.

    Hope this helps, happy coding :)

    Marked as helpful

    0
  • Esteban Reyes• 40

    @Estebankdb17

    Submitted

    Hi everyone!

    Don't know why but this challenge was harder than I thought, but I did it. I'll apreciate any suggestion to improve my code. The font weight in pharagraphs is not the correct because when I put it in 400 the last "Learn More" button go away to the top of viewport, and I can't find the mistake or any solution to this :(

    Thank you!

    Responsive solution using HTML/CSS only

    2

    Agata Liberska• 4,015

    @AgataLiberska

    Posted

    Hi @Estebankdb17, really nice work here, it's great that you remembered your landmark elements. Two things jump out that can be corrected relatively quickly:

    • In mobile layout, the border radius is exactly what it is in desktop layout - if you wrapped all three cards in a container, you could set the border radius on that container which would do the job for both column and row layout (you may need to add overflow: hidden to get it to show up)

    • In mobile layout, the footer is sort of in the middle of everything, can't figure out quite why right now, but - you could get rid of position: absolute on your footer, and use flex box properties to get it to stick to the bottom of the screen. To achieve this, you would need to set the body to flex-direction: column;' so that the mainis on top of thefooter. Then you need to make sure that the mainelement expands to fill all available space usingflex-grow: 1` (you may also need to set the max-width to 100% to get it to collapse correctly.

    • I think you could also simplify your markup and styles a bit - for example, your learn more anchor tags don't really need to be contained by anything, definitely not a section element, which really doesn't need to be a grid container.

    Hope this helps, let me know if you have any questions :)

    Marked as helpful

    1
  • Agata Liberska• 4,015

    @AgataLiberska

    Posted

    Hi @hariprasad9899, nice work, just needs a few tweaks to match the design better.

    • In most cases, you don't need to set the height of an element at all. As you can see in the screenshot above, on large screens your card is a lot bigger than it should be and it has of empty space. If you just remove the height rules of your .container, this problems go away - it's ok to make the content (+padding) decide the height of an element and just control its width. Really, I would remove all height-related rules in your solution.

    • If you start working on a challenge from the mobile design, you can make things simpler for yourself - for example, in column layout, you wouldn't need the flex container at all - this property could only start applying from 800px and wider.

    • Try to remember to include landmarks in your solutions - here, you could add a <main> tag as the direct child of the body. You could also use a few more semantic tags, like <article> and <section>.

    Hope this helps!

    Marked as helpful

    0
  • drunken_neoguri• 290

    @DrunkenNeoguri

    Submitted

    (I wrote this using Google Translate. Please forgive the grammar.)

    I did this project as a second challenge!

    I studied the use of 'hover' in a previous project, so I was able to work relatively quickly. However, while writing the code, I thought that there might be a way to simplify the code a bit more... but nothing came to mind.

    And I tried using 'inherit' to apply the 'width' value of the 'body' tag to the image in mobile size, but it was not applied the same as the 'width' value of the body. T_T)

    How should I write it to apply the same?

    Also, if you have any other advice, please feel free to tell us! Thank you for watching! :D

    2nd Challenge: Order Summary

    1

    Agata Liberska• 4,015

    @AgataLiberska

    Posted

    Hi @DrunkenNeoguri, my question here is - why do you need to set the body's width at all? I'd leave it alone and allow it to take up the default full width of the screen, and just set a max-width on the card.

    Other than this though, nice work on this challenge, it looks very nice, maybe you could try to add the box shadow under the blue button to match the design? It would also be great to see more semantic elements :)

    Marked as helpful

    2
  • Virginie L• 10

    @VirginieLemaire

    Submitted

    Hi, I'm not sure I choosed the right semantic...

    For the mobile view, my elements are centered in the navigator mobile view but when I use an extension they are aligned to the top ?!? Any idea why? Thanks a lot

    Stats preview card HTML/CSS vanilla using flexbox

    3

    Agata Liberska• 4,015

    @AgataLiberska

    Posted

    Hi Virginie!

    When it comes to project like this, where you only have a single card etc. I like to think of it as an element taken out of a bigger project rather than a big project in itself. So I personally would choose to have a <main> element as a first child of the <body>, and then put my card in that. I think for a card, <article> is often a decent choice and it definitely would suit here, you can also have <sections> within the article. The list of stats would be a good opportunity to use <ul> as it is a list, even though it's styled to have list items next to each other.

    One thing I would correct in your solution - I would modify the media query so the card is in a column layout for a little bit longer, there isn't quite enough space for the two halves to be next to each other at 673px. Also, the background image repeats at this width - would be good to just set it to no-repeat :)

    Hope this helps!

    Marked as helpful

    2
  • Karolis• 100

    @KarolisGaiv

    Submitted

    1. I was not sure how to add error icon into input field. I've done it through CSS by using "background-image" and "background-position" properties but was wondering how good this solution is? Is it a valid and common way to do such thing or is it just some spaghetti code?

    2. Even though this is quite simple form and it's validation should not be too complex but I didn't follow DRY principle. My other ideas of how to write a form validation seemed to complex (way too many lines of code, multiple functions). How important is code quantity? Is it better to have such solution as mine (where I keep repeating code over and over but it's quite simple to understand) or have multiple functions which are called maybe once or twice but the code is not so repetitive?

    Thanks for the feedback.

    Mobile-first Intro sign-up form

    #accessibility

    2

    Agata Liberska• 4,015

    @AgataLiberska

    Posted

    Hi Karolis! :)

    1. I would normally add the icon in the html and then position it absolutely within a form-control div that also has the input and related label in it.
    2. I don't think this is a massive issue here where you don't actually have that much code in general, but overall you could try to group things together. For example, instead of having checks for each field separately, check all inputs at once and pull out input name for the error message.
    3. it would also be great if the error disappeared as soon as the input field had correct value in it.
    4. And just visually - would be great if you added a max-width to your page content - on my screen things stretch out a bit too much :)

    Marked as helpful

    1
  • Agata Liberska• 4,015

    @AgataLiberska

    Posted

    Hi Rodrigo, this looks really nice! The one thing I'd change is instead of setting a width to the cards, set a max-width. It will still look the same on wider screens but will also help the content not overflow on smallest viewports :) You could also adjust the padding on the cards a little bit to make them match the design a bit better, but the layout was the biggest challenge here and you dealt with it really well.

    Marked as helpful

    2
  • Agata Liberska• 4,015

    @AgataLiberska

    Posted

    Hi @SAAJEVES, I had a look as requested in your comment and I've got some suggestions. I would modify your media query to not switch to desktop layout so soon - 425px is definitely not enough space to have the image and text next to each other.

    In desktop as well, I would suggest setting a max-width to all content (so on your header and main). I think it will really benefit the design - on my screen, the image is really big and it pulls attention away from the text - there just isn't much content available to balance it out.

    Also, you don't need to make sure that the text wraps in the same space as it does on the design (maybe other than the main heading, the breaks will usually add some visual interest and impact to them). You can let the other content flow more naturally, as long as it matches the design (more or less, depends who you ask) at the given widths.

    Hope this helps :)

    Marked as helpful

    0
  • Rodrigo Nascimento• 220

    @rodrigovn

    Submitted

    This was a challenge for me because I tried to adapt to 3 different screens: iphone, ipad and desktop. I need some feedback because I think I took too much time in this one. I had some issues in positioning my popup message, and adjusting in code to different screens.

    Article preview component challenge using flexbox

    1

    Agata Liberska• 4,015

    @AgataLiberska

    Posted

    Hi Rodrigo! I remember having quite a few issues with this challenge too, it's quite a tricky one!

    It's definitely a good idea to make sure your solutions are responsive from the very first challenge you attempt, so nice work here :)

    I see there is an issue with the image in desktop view. The problem is this: your .card div is a flex container, and you've set align-items: center. This is why the height property on the image-wrapper isn't doing much. However, if you change align-items to stretch it will fill up the full height of the div.

    The problem then will be that the image is going to be stretched out, but object-fit: cover on the image should fix it :)

    And when it comes to positioning your popup box - remember that you must set position: relative (or anything other than static) on a parent of the absolutely positioned element - otherwise it will be positioned in relation to the body which is hard to manage :) so if you add this to your .container or the .card, things should be easier!

    Hope this helps :)

    Marked as helpful

    0
  • Agata Liberska• 4,015

    @AgataLiberska

    Posted

    Hi @hyde-brendan, really nice work with the card and the shadow looks great. Have you seen Josh Comeau's article on shadows and his shadow generator? I mean this one: Designing beautiful shadows in CSS It is very in-depth but it's a delight to read :)

    One thing I noticed in your solution that could be corrected - on very small mobile size (around 320px), the card stops fitting on the screen nicely and you can see the background pattern ends before the edge of the screen - I think it's the div with price info not wrapping that causes this. I mean this would be an edge case but it would be nice to have this covered - maybe you could display the icon only at like 350px?

    Hope this helps :)

    1