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

All comments

  • CY Chan• 290

    @CYCHAN00

    Submitted

    Please do not hesitate to tell me if you have any comments, suggestions, or improvements ~ Thanks

    Single price grid component

    1

    P
    Dave• 5,245

    @dwhenson

    Posted

    Hey @CYCHAN00 👋

    Lovely job here, and not much to comment on 🙌, but here's a couple of things to think about:

    • Button vs link? One thing that's worth considering is whether an element is actually a button or a link . The most important thing is what the element will do not what it looks like. This page has a great summary and lots of useful links on this: https://css-tricks.com/buttons-vs-links/ I imagine clicking on 'sign up' would take me to a new page, if you also think that then it should be a link.

    • I'd also get rid of all those br tags inside the square on the bottom right and just put everything inside a ul and make each item an li. I think this would be more semantic as they seem to be a list to me. You will need to remove the default list styling, but once that's done things will appear as they are now, but be better for people using assistive tech to access your site.

    Otherwise, keep up the good work and great job! 🥳

    Cheers Dave

    0
  • P
    Dave• 5,245

    @dwhenson

    Posted

    Hello Ahmend, 👋

    Nice work here! This is a tricky one, you've got some of the hard details like the top border sorted - that really gave me problems for a long while! Here's a few things you might like to consider:

    • You are using grid on the main section, which is great, but not really getting the maximum benefit from this property. If you're new to it I'd suggest checking it out as it can be super helpful.

    In this case, you are using grid, but then also using margin to position elements, I'd go for one or the other (in this case grid most likely!). I'd suggest trying a grid with 4 rows, and 3 columns. And then have each card span the two rows it needs to (so the top card would be rows 1 and 2, and the middle cards 2 and 3, and the bottom card rows 3 and 4 - you'll also need to specify which column they should go in).

    Then if you use the gap property, rather than margin you should have a nicely laid out desktop page. You can then change the grid layout with a media query for mobiles (I'd actually probably start with the mobile view and then put the settings above in a media query for desktop, but it doesn't matter really).

    This is a bit of a tricky one to explain, but I hope you get the gist of what I am trying to say. If not just let me know and I'll try and clarify!

    Cheers Dave

    Marked as helpful

    1
  • P
    Dave• 5,245

    @dwhenson

    Posted

    Hey @Someshwari - Lovely job here.

    The site looks great and responds well. Here's a couple of points to think about:

    1. You are getting some error messages from the linter about missing heading levels. This is because you should use the <h1> headings as the single, main heading of your web page, followed by the <h2> headings, then the less important <h3> headings, and so on, without skipping a level.

    We shouldn’t use headings to make text look BIG or bold. Use them only to set out your document's heading and show the document structure, and then change things up with CSS after that.

    I approach this by first laying out the page using only HTML and only thinking about the document structure, not design at all, and then once done, I return to the page and use CSS to make things look how they should.

    This is important as many people using assistive tech to access your pages will navigate the site based on the heading structure. At the moment this wouldn’t work with your HTML.

    1. I wonder whether your button should actually be a link. I imagine that it will take me to another page to read more? If so it should be a link. Again, the most important thing is what the element will do not what it looks like. This page has a great summary and lots of useful links on this: https://css-tricks.com/buttons-vs-links/

    Hope this helps a bit and gives you something to think about for the next challenge!

    Keep up the good work. Dave

    1
  • P
    Christopher Adolphe• 580

    @christopher-adolphe

    Submitted

    Hello frontend friends! 👋

    This was long overdue but I finally completed my 4th challenge on Frontend Mentor. 🎉 I have been learning React since the beginning of 2022 and this project was a great opportunity for me to put what I've learnt into practice. Check my readme.md for more details.

    Major challenge(s):

    • I decided to use React with Typescript and I had some difficulties in getting the right types for components props.
    • I really wanted to get the real development experience so I added unit tests for the components and end-to-end tests for the order scenarios.
    • I have yet to add unit test for the custom hook.
    • I still have to figure out how to set the redirection properly for a React application on Netlify.

    Bonus:

    • I added some form feedback if the user tries to create a plan without selecting any options.
    • I added a skeleton preloader while the page content loads. I always wanted to implement such a feature and therefore I tried to come up with a solution for that using React.

    It took me quite a while to complete this challenge because I was going back and forth in the React documentation and best practices. I really enjoyed that and I'm also happy with the result.

    Your feedback would be much appreciated.

    Thanks in advance. 🙏

    Coffeeroasters subscription site | React, Unit Tests, End-to-End Tests

    2

    P
    Dave• 5,245

    @dwhenson

    Posted

    Hi Christopher,

    Lovely, lovely job here! I'm also pretty new to React and have been learning it for a few months so it's nice to find a fellow student.

    I noticed a couple of problems with the final modal 1) Keyboard focus doesn't move there and 2) The body remains scrollable.

    I've been lucky to be an early test on Josh Comeau's Joy of React course and just yesterday did the modal lesson. It can be really hectic, but there's a couple of packages that can basically solve this for you:

    1. react-focus-lock - moves and locks focus to the modal
    2. react-remove-scroll - removes scroll from all other elememts

    I've not tried them on a project but they seem pretty simple to implement and it might be worth giving them a try?

    Cheers Dave

    Marked as helpful

    1
  • P
    Dave• 5,245

    @dwhenson

    Posted

    Hi Isaque803

    Nice job here! Here's a few points you might like to consider:

    1. You could make the burger-menu element a button rather than a div. This will ensure that it can be opened by keyboard users.
    2. Perhaps have a look at the srcset attribute? This will means you won't need to always load two images to allow for wide/narrow screens, which is much better for performance.
    3. I think you need to put an accessible label on the svg icons at the bottom of the page? You could use aria-label (I think they should also be links?).

    Otherwise nice job on this one!

    Cheers Dave

    Marked as helpful

    1
  • Miran Legin• 680

    @miranlegin

    Submitted

    Hi all,

    this challenge was a fun one and i'm gonna try and explain my process.

    I've used Astro on this project for building .html files. I know this is kinda overkill for this challenge but i'm lazy with syncing individual .html files especially repeating sections like footer etc...

    Next i decided to create a custom map on Location page with Mapbox as i have some experience with it. It can be improved for sure but for this example i think it looks ok. I also added custom Pin from design and added Popup functionality as well.

    For layout i've mostly used CSS grid with some Flexbox along the way where needed.

    Images are marked with Picture element with multiple Source's for responsiveness. I've also added @2x versions where needed.

    Lastly i've played with :hover state on buttons with @media(hover) to only target supported devices.

    Let me know what you think!

    Cheers, Miran

    Build with modern CSS, Astro SSG and Mapbox custom designed map

    1

    P
    Dave• 5,245

    @dwhenson

    Posted

    Hi Miran - another lovely and beautiful job here!!

    Well done on using Astro, I'm keen to try and have a go at using it one day. I love the transitions you've added to the buttons, very nice!

    I've very little I can add or suggest here. One thing is that when my mouse is over the map and I scroll the map zooms in and out, and I'd prefer to just scroll - this is especially the case on smaller screens where the map takes up more of the screen (but this is really a small issue).

    On a much more general level, you might like to have a look at some of the approaches set out in every-layout.dev. Some of it is pretty advanced CSS layout stuff, but I don't think it will be too difficult for you! I love the approach to a more 'intrinsic design' approach, and it reduces a lot of the need for media queries.

    Here's my solution of the same challenge that uses some of these approaches: https://www.frontendmentor.io/solutions/art-gallery-page-more-intrinsic-design-ESLbwbiu3t (my solution is no way as nice as yours, but I think I pretty much avoided break points completely in this challenge to create a more 'fluid' solution).

    Obviously, this just depends on what kind of approach you'd like to use for CSS, but I've come to really like this way of doing things.

    Keep up the lovely work! Very inspiring for me!

    Cheers Dave

    1
  • Vasile Cosmin• 160

    @VasileCosmin

    Submitted

    This is my solution. I don't know why it doesn't give new advice everytime I click the button, but gives the same advice. Can you tell me what I did wrong? Thank you:)

    CSS Flex Javascript Fetch

    2

    P
    Dave• 5,245

    @dwhenson

    Posted

    Hi Vaslie

    I just did a quick test and it seems to be working fine for me! Nice job. Here's a couple of points to consider:

    1. For the button, you should probably add an aria-label to help people with screen readers understand what the button does
    2. If you really want to help them I would also add an aria-live="polite" to your h1 as this will announce the text when it is rendered to screen readers too.

    Otherwise I think this looks good. I like the way you included some default text for people to read while the first fetch is happening, and that you included a catch in case of errors.

    For the return of data I typically do something like response.ok ? response.json : new Error. This also checks the returned data is fine, and adds another level of robustness to the app.

    Hope this helps and nice work!

    Cheers Dave

    Marked as helpful

    0
  • Gabriela• 100

    @ga-bri-ela

    Submitted

    Hi everyone!

    This was my first time working with an API independently (without following a tutorial). It was quite straightforward and really helped me build confidence to take on bigger challenges with APIs.

    My biggest struggle was with the positioning of the button. I am not really happy with how I solved it, and I will look into other solutions to find something more efficient. If you have any advice, I am happy to hear it.

    Any feedback is welcomed and appreciated. Thank you in advance!

    Responsive Advice Generating App with HTML, CSS and JS

    2

    P
    Dave• 5,245

    @dwhenson

    Posted

    Hello Gabriella,

    Lovely job here! In response to your button challenge here is what I would do:

    1. Remove the fixed height from the square-background. I would suggest in general to avoid setting heights if possible and use content, and if needed padding, to create a larger element.
    2. On the button, remove the absolute positioning, and add transform: translateY(50%), and that should do it.

    Transforms to position an element are great as they don't impact performance, and unusually for CSS, the percentage refers to the element being targeted, not its parent.

    So in this case we are telling the element to move down 50% of its height. Which is about where you want it I think? This also avoids hard coding pixel values which can make responsive design pretty tricky.

    Hope this helps and nice work on the fetch call. Nice that you got some JS working!

    Cheers Dave

    Marked as helpful

    1
  • Joanna Lee• 140

    @doleetos

    Submitted

    Holy moly, this took quite some time to finish with ReactJS. But I had SO much fun! Please, please leave me any comments or feedback to better my code as I'm still new to React. I'm going to continue working on this to add more styles and make the site more interactive.

    Sunnyside agency landing page solution

    1

    P
    Dave• 5,245

    @dwhenson

    Posted

    Hey Joanna

    Lovely job here! The page looks great and responds well. I'm also learning React at the moment, but you are clearly way beyond me!

    Only a couple of small comments: as per the HTML warning I would put the testimonials section inside the main element, and probably make that sections heading a h2 (generally only one h1 page is advised).

    The only other improvement I could possibly suggest is to use pseudo-elements for the link underlines? This will give you a bit more control over shape and positioning (I am working on this challenge at the moment and used this approach - but not using React!)

    Lovely job and hope this helps a little.

    Cheers Dave

    Marked as helpful

    1
  • Goran• 500

    @GoranK89

    Submitted

    This was a fun one to explore async functions and work with JSON data. The columns are dynamically drawn based on the JSON file, and the active column is also based on the current day. Any suggestions are always very welcome. 🙂

    Expenses chart component - vanilla JS

    1

    P
    Dave• 5,245

    @dwhenson

    Posted

    Hey Goran, lovely job here! The component looks great and renders really fast which is great. Here's some points you might like to consider:

    • Heading order: In HTML we shouldn't skip heading levels (which I don't think you have done) or use heading levels for presentation purposes (which I think you have done? Using a h2 to make the price font larger for example).

    This is important as many people using assistive tech to access your pages will navigate the site based on the heading structure. At the moment this wouldn’t work with your HTML. The would probably need to skip to "my balance" for example, before the actual number.

    I approach this by first laying out the page using only HTML and only thinking about the document structure, not design at all, and then once done, I return to the page and use CSS to make things look how they should.

    We shouldn’t use headings to make text look BIG or bold. Use them only to set out your document's heading and show the document structure, and then change things up with CSS after that. It's an easy thing to do if you start out with this mindset, and can really help people.

    • getElementsByClassName is perfectly valid, but using class names to select elements has really bitten me a few times. They change so often, and get toggled on and off and all sorts of things. I'd suggest perhaps using an id or even a custom data- attribute to hook into the HTML. I do this just as standard now as using classes has given me too many headaches!

    Here's a nice article on the approach: https://gomakethings.com/strategies-for-working-with-data-attributes-in-vanilla-javascript/

    My only other small suggestions would be to include some error handling with a catch statement in your async functions, and maybe check response.ok is true before continuing. These will both just improve things when things don't work. This property is the best one to check when receiving data (as I understand it!)

    I hope this helps and bit and keep up the good work!!

    Cheers Dave

    Marked as helpful

    0
  • Rizwan Khan• 130

    @mdrizwanfk

    Submitted

    I feel I have extensively and conveniently used Flexbox across this project.

    Q1. Can any flexbox alternatives be more efficient in any of the scenarios? (Below is the list of places I used flexbox:

    1. Alignment of Discounted and MRP prices.
    2. Within the Call-to-action Button for the Icon and Text's Alignment.
    3. In Desktop Design, Centering the Card Component vertically and horizontally.
    4. In Desktop Design, Flexing Image and Description within card to Row direction.
    5. In Desktop Design, Spacing a Column Flex of Card Description elements.)

    Q2. Suggestions/Improvements to structure responsive bits in the code. (style.css)

    Q3. I have not used img HTML elements for the Card image throughout this project, I used background-image CSS Rule instead, Will this impact SEO and Search results? And is my implementation considered as best practice in the industry?

    Q4. Is there a more elegant way to Round corners of our divs, in this case. I had to repeat border-radius at 2 places, one for image div and the other for the card. Can this be improved?

    Thanks for reviewing, in advance. Stay safe & take care :)

    Responsive card using Flexbox and Media Queries

    1

    P
    Dave• 5,245

    @dwhenson

    Posted

    Hey Rizwan, lovely job on this one, the component looks great and responds well. 🥳

    Here's some general points to consider:

    • Check the headings issue in the accessibility report. What this means is that you should use the <h1> headings as the single, main heading of your web page, followed by the <h2> headings, then the less important <h3> headings, and so on, without skipping a level.

    This is important as many people using assistive tech to access your pages will navigate the site based on the heading structure. At the moment this wouldn’t work with your HTML as you skip and level and use a h4 (presumably as it's smaller?).

    • I'd add a hover state on the button, and change the curser to pointer. I think strictly speaking we shouldn't but it's kinda expected behaviour now.

    • The entering of your component appears to be in a media query? As such at smaller widths it pops up to the top of the viewport. I'd suggest you just keep the body with display flex throughout (I often just use grid and place-items: center).

    With regard your questions,

    Q1 - You could perhaps inline the svg and then use display: inline-flex; just saves one network request, but not a big deal!

    Q2 - This can be a bit tricky, but I would try and do without the breakpoint to change from columns to row flex direction, and use flex-wrap and a min-width on either the the preview or details elements. Check out Every Layout by Andy Bell and Heydon Pickering for more o this and how far you can take flex box!

    Q3 - I would probably use a img on this one as there's no text over the image, and it is directly related to the text. I'm not sure about accessibility and CSS background images, but the img approach works. Also saves having an empty div, which is always a bit of a warning to me.

    Q4 - You could use a utility class for this perhaps? The other option could be to set the radius as a custom property? That way you can at least adjust the size easily, if not the application!

    Hope this helps a bit, lovely job over all!

    Cheers Dave

    Marked as helpful

    1
  • P
    ania• 330

    @ania221B

    Submitted 

    /* Remove list styles on ul, ol elements with a list role, which suggests default styling will be removed */
ul[role='list'],
ol[role='list'] {
  list-style: none;
}

    Yet setting role to list is treated as a mistake in the report. Is it a bad idea to use role="list" to remove default styling?

    • There was no tablet view. It doesn't seem necessary but should it be added somehow anyway?

    3 column preview card using grid, flexbox and custom properties

    2

    P
    Dave• 5,245

    @dwhenson

    Posted

    Hi, just to add that there's a "bug " or rather a decision made by Safari that removes some of the functionality from lists that screen readers need, details here: https://www.scottohara.me/blog/2019/01/12/lists-and-safari.html

    I use this reset, and just accept that the HTML linter will flag issues each time. This is a bit of a tricky one, and I don't know how to call it, so in the absence of better guidance, I just follow Andy's suggestion and add the attribute.

    Marked as helpful

    1