Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Your session has expired please log in again.
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

    #react#tailwind-css#next

    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

    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

    #cypress#react#react-testing-library#sass/scss#typescript

    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
  • @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

    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
  • @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:)

    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!

    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.

    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. 🙂

    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
  • @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 :)

    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 300

    @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?
    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
  • P
    Dave 5,245

    @dwhenson

    Posted

    Hi @nikoescobal - lovely job here! 🥳

    I think if you add the following rules to the footer you should get the result you want, I just tried it in the browser and it seems to work (perhaps you were missing the background-position part?):

        background-repeat: no-repeat;
        background-position: center top;  
        background-size: cover;
    

    Otherwise, 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 would think in this case the "download" could be a button, but the other "What is it" element should perhaps be a link? All this is debatable of course, the main thing is to be aware of the different use cases.

    Keep up the great work!

    Cheers 👋

    Dave

    Marked as helpful

    1
  • P
    Gareth 380

    @Gareth-Moore

    Submitted

    So this wasn't very difficult. But I'd love to know what you think of the API process I used and any advice regarding the JS/API.

    Also... this website works perfectly in Chrome, but in Firefox it returns the same advice even if I close and reopen the browser. It's a bit odd and almost impossible to find any solutions to fix it. If you do know, I'd love to hear about it.

    Thanks, Gareth

    P
    Dave 5,245

    @dwhenson

    Posted

    Lovely job here. The app works well 🙌

    I would just suggest in your JS to add a check to ensure the API has responded correctly and render an error message if not. This isn't too tricky, and I would just change the first then to be something like:

      .then(response => response.ok ? response.json() : Promise.reject("API failed")```
    

    If you then add a catch at the end of your fetch function you can render some fallback text in case things don't work for any reason.

    It's not a big deal, but thinking about how things can go wrong is a good habit to get into.

    Cheers 👋

    Dave

    Marked as helpful

    1
  • DanMo 120

    @DannyMolina

    Submitted

    Someone who can guide me, I managed to make the design for mobile phones but when it comes to making the desktop I get disorganized I don't know where to arrange them use grid and it's a disaster hahaha someone who can help me

    Edit: I see that it disorganized me in the naming of the classes and I assigned a class that harms everything, I do not want to throw more head at this so that is how I am crooked

    P
    Dave 5,245

    @dwhenson

    Posted

    Hey @DannyMolina - nice work here! The page looks good. I use a few tricks to deal with the issues on your page, it's worth finding an approach that works on these points as they come up a lot! Here's what I would to do improve things:

    1. Prevent things spreading out too wide on large screens.

    There are many ways to do this but I set a grid on the body element, with three columns, as using a class selector as follows:

    .center-content {
    	display: grid;
    	grid-template-columns: minmax(1rem, 1fr) minmax(375px, 1440px)minmax(1rem, 1fr);
    }
    
    .center-content > *  {
    	grid-column: 2;
    }
    

    The 1440px is the max-width you want the main content to be, and the 1rem values is the smallest spacing you want either side of the main content on small screens (I sometimes put this to 0 and use a container to add padding to each section).

    The second part positions all direct children of the body in this nicely sized middle column. In my case, mostly, my header, main, and footer the middle column, and stops them going wider than 1440px. It’s also pretty easy to ‘break’ elements out of this constraint if you need to.

    Other people use container classes to do the same thing. This article has a good run down of alternative approaches https://css-tricks.com/the-inside-problem/ You will note I am actually using the approach the author doesn't like! I am thinking of switching to the container approach.

    Either way it's a good idea to find an approach that works for you as you'll need this for a lot of FEM challenges.

    1. Change the alignment of the buttons.

    This can be done with flexbox. Firstly I would wrap the input and button in a div. And set the following on the div:

    display: flex;
    flex-wrap: wrap;
    gap: 1rem;
    

    I would then apply the following to the input:

    flex-basis: 0;
    flex-grow: 999;
    min-inline-size: 70%
    

    This combined with the flex-wrap on the parent means that the input will grow and shrink, but when the width hits 70% the button will wrap below. Then on the button add:

    flex-grow: 1;
    min-inline-size: 20rem;
    

    This means the button will not be less than the size set (you could also use fit-content if you are feeling fancy!) I think that should work. You may need to play around with the min-inline sizes.

    1. Lastly you should add accessible name to the links listed in the report. This is probably most easily done by using aria-label. You can't have empty links with no accessible name as screen readers won't announce where the link will go.

    Hope this helps!

    Cheers

    Dave

    PS If that flexbox stuff is a bit tricky, you could always use a media query and just change the flex-direction from row to column at narrow widths.

    0
  • P
    Dave 5,245

    @dwhenson

    Posted

    Hey @Gareth-Moore lovely job here! 🥳 The component looks great and I don't have much to add! Just a couple of small things to think about:

    1. Is that a button or a link? I'm not sure of the answer, but as long as you are not deciding based on how it looks you are good! This page has a great summary and lots of useful links on this: https://css-tricks.com/buttons-vs-links/

    2. Either way, it might be nice to practice adding some hover styles to the button/link, and adding "pointer: cursor". The last point is a bit controversial for a button, but I think most sites add it. Both of these are just to give the user some hints that they can click.

    But as above, small point and lovely job! Keep it up and good luck with the next one!

    Cheers 👋

    Dave

    Marked as helpful

    0
  • P

    @mdubelbeis

    Submitted

    This project offered some challenges. Most of the challenges were self-inflicted but I was able to find work arounds to get the task complete. I will continue to correct these as time goes on.

    One of the areas I am not happy about is the responsive images and not being able to implement srcSet. This will be one of my goals because this seems to be one of the regular issues I have.

    Sunnyside-agency-landing-page

    #react#tailwind-css

    1

    P
    Dave 5,245

    @dwhenson

    Posted

    Hey @mdubelbeis - I think you need to remove overflow-y: hidden from the body! Otherwise, it's looking good from my quick review!

    Cheers

    Dave

    1
  • ROCKY BARUA 1,090

    @Drougnov

    Submitted

    I need some help on these issues:

    • What is the best approach/way to make the background pattern pixel perfect as shown on the design?
    • The z-index is not working on the 'quote' in the review section. How to fix it?
    • Is there a way to animate the bottom part (when visible) with only JavaScript? Or do I need some libraries/frameworks?

    I tried to make it close to the design. Yet I might miss out on many things or do wrong in some parts. Please let me know if you notice. Thanks in advance :)

    P
    Dave 5,245

    @dwhenson

    Posted

    Hey @Drougnov lovely job here! The page looks great!! 🥳. Here's some things to think about:

    • Personally, I never worry too much about pixel perfection. I go for "pixel pretty close" here's a great article about this: https://www.joshwcomeau.com/css/pixel-perfection/

    • The quote should be on top of the review section? It is on my screen - can you clarify the issue there? Sorry if I'm missing something obvious!

    • And again for the animation - which 'bottom part' would you like to animate? You can do most things without a library, but in some cases it might be a lot of work!

    Lastly, you might want to think about stopping the page from spreading too wide a very large screens. There are many ways to do this but I set a grid on the body element, with three columns, using a class selector as follows:

    .center-content {
    	display: grid;
    	grid-template-columns: minmax(1rem, 1fr) minmax(375px, 1440px)minmax(1rem, 1fr);
    }
    
    .center-content > *  {
    	grid-column: 2;
    }
    

    The 1440px is the max-width you want the main content to be, and the 1rem value is the smallest spacing you want on either side of the main content on small screens (I sometimes put this to 0 and use a container to add padding to each section).

    The second part positions all direct children of the body in this nicely sized middle column. In my case, mostly, my header, main, and footer are the middle column and stops them from going wider than 1440px. It’s also pretty easy to ‘break’ elements out of this constraint if you need to.

    Other people use container classes to do the same thing. This article has a good run down of alternative approaches https://css-tricks.com/the-inside-problem/ You will note I am actually using the approach the author doesn't like!

    Either way, it's a good idea to find an approach that works for you as you'll need this for a lot of FEM challenges.

    Cheers

    Dave

    Marked as helpful

    1
  • @BhekiAccion

    Submitted

    Dear coders, kindly advice me here, how do l change the color of an SVG image? You will notice that l struggled to change the colour of the logo picture at the footer section. Anyone with knowledge or an informative link?

    P
    Dave 5,245

    @dwhenson

    Posted

    Hey @BhekiAccion, nice work here!

    So to manipulate SVGs you have to 'inline' them in the HTML document. That is, actually paste the SVG as code. From there you will be able to target the different elements inside the SVG itself, if you add an SVG as an image as you have, then it's impossible to change the colour on hover etc.

    It can look a bit intimidating the first time you do this, but it's worth getting comfortable with doing this and targeting the relevant elements inside the SVG itself. It's also better for performance so you get a bonus of doing things this way. I find it helpful to think of the SVG as a document in itself.

    Hope this helps,

    Cheers

    Dave

    Marked as helpful

    1
  • P
    Dave 5,245

    @dwhenson

    Posted

    Lovely job here! The page looks good and responds well....

    You might want to think about stopping the page from spreading too wide a very large screens. There are many ways to do this but I set a grid on the body element, with three columns, as using a class selector as follows:

    .center-content {
    	display: grid;
    	grid-template-columns: minmax(1rem, 1fr) minmax(375px, 1440px)minmax(1rem, 1fr);
    }
    
    .center-content > *  {
    	grid-column: 2;
    }
    

    The 1440px is the max-width you want the main content to be, and the 1rem values is the smallest spacing you want either side of the main content on small screens (I sometimes put this to 0 and use a container to add padding to each section).

    The second part positions all direct children of the body in this nicely sized middle column. In my case, mostly, my header, main, and footer the middle column, and stops them going wider than 1440px. It’s also pretty easy to ‘break’ elements out of this constraint if you need to.

    Other people use container classes to do the same thing. This article has a good run down of alternative approaches https://css-tricks.com/the-inside-problem/ You will note I am actually using the approach the author doesn't like!

    Either way it's a good idea to find an approach that works for you as you'll need this for a lot of FEM challenges.

    Otherwise, I can see the challenge you are facing with the alignment of the error message on the CTA. One solution could be to wrap the input and the error message in a parent and use this as the basis for positioning them relative to each other?

    Again, this is a common pattern in many FEM challenges so it's worth spending a bit of time working out a way that works for you and you can apply in the future.

    Hope all that makes sense, but let me know if anything isn't clear!

    Cheers

    Dave

    1
  • Anna Leigh 5,135

    @brasspetals

    Submitted

    Hi, everyone! 👋 It feels great to finally submit this solution. I’ve wanted to do this challenge since I started doing Frontend Mentor projects. It was definitely a learning experience (see: README), and hopefully I’ve made up for the lack of effects in my Sunnyside solution. 😆

    The parallax effect is only on desktop and tablet. It’s also turned off, along with all animations and page transitions, for those who prefer reduced motion. 👍

    Questions:

    • This was my first time implementing a “tabbed” interface. I opted to use button elements for the tabs. Is there a better and/or more accessible way to create tab functionality?
    • I’m concerned that the image animations on the tabbed Events section are getting messed up if the image doesn’t load before the animation triggers. Is there a way to preload images from picture elements in a way that’s responsive and also works with multiple srcsets? Update: I refactored this section a bit, making some tweaks to the animations and putting all content in the DOM rather than dynamically loaded. The flashing issue is now fixed. Huge thanks to Dave and Christopher for their help on this! 🙏
    • How is the form accessibility on the booking page? I don’t believe the second page was included in the report.
    • Sadly, I didn't manage to style the AM/PM selector like the design because I used select and option elements. Do you know of a better method that would be both accessible and match the dropdown in the design?
    • Not sure how I feel about the menu list being parallax on desktop. Does it look too wonky or have I just been staring at it for too long? 😂

    Thanks for taking the time to look at my solution! Feedback is always greatly appreciated. 😄

    P
    Dave 5,245

    @dwhenson

    Posted

    Hey Anna - lovely job as always.

    This is my go to resources for tabbed interfaces: https://inclusive-components.design/tabbed-interfaces/ - as ever with Heydon, very in depth.

    When finding this I also saw this link which might be of interest? (I don't know Svelte so not sure how good it is.) https://github.com/aral/heydons-inclusive-tabbed-interface-in-svelte

    One small issue on the tabs is that I sometimes get a flash of the previously rendered text or photo when selecting different options. Just a small point though.

    Cheers Dave

    Marked as helpful

    2
  • P
    Dave 5,245

    @dwhenson

    Posted

    Hey Anas - lovely work on this challenge - it is quite a tricky one!! 🙌

    I would suggest the following improvements could be made to your JS;

    • I would include some input validation to check the value entered is a URL. At the moment I can try and enter other values and I don't get an indication that this isn't allowed - this will provide some quick and instant feedback to the user
    • On successful return of the API call, I would also suggest reverting the input to an empty state - just a nice thing to do for users
    • In the API call, I would include a catch statement to collect any errors - these can then be shown to the user. This is in case the URL submitted was in the proper format, but for some reason, the response didn't work out.

    If you check the response in the dev tools you can see that the response from this API has some useful error messages that can be shown to the user.

    Hope this helps a little!

    Cheers Dave

    Marked as helpful

    0
  • @Jeth0214

    Submitted

    Hello Guys, Is it good to have this project as my portfolio? I am just going to change those images and edit the descriptions and links. Can you guys give ideas on what I need to improve or change if I will going to use it as my portfolio.? Also, please have a code review on my solution. It will help me write my code more efficient and readable. Thanks.

    Minimalist Portfolio using HTML, CSS, Javascript

    #accessibility#fetch#sass/scss#semantic-ui#solid-js

    3

    P
    Dave 5,245

    @dwhenson

    Posted

    hey @Jeth0214, the site looks good!

    Definitely look into using a HTML solution for managing your images! My only other suggestion is to perhaps put a min-height on the body as in the contact page your footer is no longer at the bottom of the page which looks odd.

    I actually adapted this challenge for my only site, it still need work, but you can have a look here - https://dwhenson.com/ you might get some ideas of changes you could make to give it your own style.

    Cheers Dave

    Marked as helpful

    1
  • @GrzywN

    Submitted

    I improved my previous solution by improving almost everything. Now with figma and better experience I was able to create pixel perfect or almost pixel perfect solution to this challenge and I'm really proud of it.

    Things I included in this solution:

    • On load animations for image and text
    • QR code popup with black glassy background (with animations)
    • Better responsiveness (it's not perfect)

    To open the popup, you need to click on the image. To close it, you need to click anywhere outside the image.

    I learned a lot about animations and trasitions doing this challenge and I recommend you guys to try making more and more fancy animations in your future projects.

    Any feedback is welcome here!

    P
    Dave 5,245

    @dwhenson

    Posted

    Nice one! The solution looks great - here are a couple of small points to consider:

    • I noted that your popup is a link element. I wondered whether this should rather be a button as it triggers an action rather than moving the user to another URL?
    • Also once the popup image is open there is no way to close it using the keyboard. A little JS might help here as otherwise keyboard users will be trapped.

    Lovely job!!

    Cheers Dave

    Marked as helpful

    0