Hey there,
This is my solution to the Huddle landing page with curved sections challenge. I've implemented it with basic HTML and CSS, no frameworks or libraries involved.
The solution is completely responsive and focuses on accessibility.
Let me know what you think :)
PS: I just noticed that the preview screenshot is broken. That might be because I used the latest CSS features, such as color-mix and CSS Nesting which might not be supported yet by the image generator. If you encounter the same issue, you might have to enable experimental feature flags in your browser.
-I don't know if I chose the best names for the classes.
-I would like feedback on the responsive part.
Hey @ricardosilvasx,
Congrats on solving the challenge, well done! Your solution looks good, I like that you wrapped everything in a main tag which is a good practice. Here are some additional suggestions:
h1) and then descend down when it makes sense. In this example, I would make the text Improve your front-end skills by building projects the main heading.div.qrcode, just apply the styles of it to the main element directly.width and height attributes, which I would advice you to set. They allow the browser to better calculate the image dimensions and hence optimize rendering, which results in a better loading experience.Keep up the good work and let me know if you have any questions!
what is the best way to handle with image and font-size RWD? I tried to limit the container to stop growing up.
Hey @MikeLee0358,
Congrats on finishing the challenge! Let me try and answer your questions:
To limit the width of the container, you can use max-width. I see that you used the clamp() function, which also works but is a little overkill for your needs. Setting max-width: 450px will prevent the container from growing any bigger but still stay flexible on smaller screen sizes.
The image inside the container is already set to width: 100%, which is great. It won't grow any bigger than the container and will shrink with it. I'd suggest adding the width and height attributes in the HTML, so that the browser knows how big the image is. This improves the loading experience, as the space is being "reserved" until the image has loaded. Also, don't forget to set height: auto in your CSS to maintain the correct aspect ratio at all times.
For this challenge, you don't need to change the font size for smaller devices if I remember correctly. So you're good already. Try to set the correct font family though, as this will improve the visuals a lot :-)
Some other things I noticed:
h1 for the heading, as it's the first and only one. Headings in HTML should start from the highest level and descend down if necessary. Since you don't have any other headings, h1 is the best choice here.div#QRCode with a main#QRCode to mark the card as the main landmark on this page. Semantics are an important aspect of HTML development and make life easier for search engines, screen readers, and others. Have a look here for more information.By the way, the method you used to align and center everything (display: grid) is really clever, nice one!
Let me know if you have any questions, keep it up!
Hi all,
All the feedback is highly appreciated.
Thank you, Giorgian
Hey Giorgian,
Congrats on finishing this challenge, your solution looks good! I found a few issues that you might want to fix:
form, which makes you lose some important functionality like pressing Enter to submit and send it. This also hurts accessibility.blur), presses the button, or even while typing (input event). So, make sure that you add an else condition in your JavaScript for when the email is valid.a element, as social links tend to redirect you to a different website, whereas buttons are more suitable for triggering interactive JavaScript.I hope my feedback helped you out, let me know if you have any questions. Keep up the great work!
Would a card component like this sit inside a html main tag, or is there something more semantically correct?
I'm not entirely happy with the margin and padding applied to the h1 and p elements. Visually it looks fine but I'm wondering would it have been better to only apply padding on these?
Hey @ricochet69!
Congrats on solving this challenge, the result looks great - it's very close to the design!
Your initial idea of using a main tag to wrap everything is good. To be accessible and semantically correct, every page needs a main landmark, which in this case is the card itself. However, I would alter your HTML structure slightly: you can remove the main.container and apply its styles to the body element. Then, you can change the div.card to main.card. This way, you save on unnecessary elements and make your code clearer while still being semantically correct.
Your margin and padding look good to me, but you could make it even simpler if you applied the padding to div.card-details. Since both the heading and paragraph are direct children, they would be moved into position by that change. I am not sure if you even need this padding, though, might be unnecessary. The margin on both elements looks good.
I am not sure why you went with height: calc(100vh - 1px) on the container? You should be able to use min-height: 100vh without the need for calculations.
Let me know if you have any questions and keep up the good work!
Hello, this is my solution to the dictionary application built with React, TypeScript, SWR, and Styled-Components. End-to-end testing handled with the Cypress testing library.
Any feedback on my project organization, testing practices, and performance would be greatly appreciated.
This project gave me a better understanding on how to use the SWR library to make http request in client components in React. I've used other libraries such as React-query before, so getting experience with other another tool was a great learning experience.
The Axios library is used to make the HTTP request in the fetch function that is called by the `useSWR' hook .
Thank you for viewing my solution, happy coding!
Hey @JorgeAMendoza,
Great job on this challenge! I played around with it and everything works as expected. I especially love the fact that you wrote good E2E test coverage and used ESLint and TypeScript. Your code is very organized and clear, the folder structure and naming are consistent and sensible. I also noticed your attention to accessibility, which is appreciated. So, very well done :)
I found a couple of minor things you might want to fix, but these are only little details:
font-context.tsx, you are using 2 contexts. If you don't think that this is necessary, you can have the font value and setter function on the same context and avoid the overhead.placeholder and aria-label. It's usually an UX anti-pattern to not have a label, but since the design is this way there's nothing us developers can do...Not an issue, just a tip: you implemented your own solution for the font dropdown, which is really good. For the future, I highly recommend using a headless library such as Headless UI or Radix UI. These libraries make it incredibly easy to build such components, because in reality it's quite hard to cover all bases (accessibility, keyboard navigation, edge collision, etc.).
Keep up the great work and let me know if you have any questions :)
I had a few issues with the API and the button while building this project. It seems to be fixed now. Any Feedback is welcomed.
Hey @andyjv1,
Nice job finishing this challenge! Since @grace-snow already mentioned some things, I'll purely focus on the React/JavaScript part:
fetchAdvice functions, one in App.jsx and the other in Dicecontainer.jsx. This is duplicated code that you should try and avoid. Ideally, you keep the fetch function inside App.jsx and trigger it with a prop like onClick, which you pass into Dicecontainer. You can then remove the fetchAdvice inside this component and connect the prop with the button's click event.fetchAdvice can be improved. You don't need a timeout to disable/enable the button, this should be depending on the state of the promise. Consider something like this:const fetchAdviceClicked = async () => { setDisabled(true); const API_LINK = "https://api.adviceslip.com/advice"; const response = await fetch(API_LINK); const advice = await response.json(); props.setText(advice.slip) setDisabled(false); };
Right before the request is made, you disable the button. Once the request went through (and assuming it was successful), you enable it again.
try/catch:const fetchAdviceClicked = async () => { try { setDisabled(true); const API_LINK = "https://api.adviceslip.com/advice"; const response = await fetch(API_LINK); const advice = await response.json(); props.setText(advice.slip) setDisabled(false); } catch (ex) { setError("Something went wrong..."); } };
disabled is truthy, it still has a hover state, which is confusing. Consider adding hover and focus states only when the button is enabled, otherwise, just gray it out. One way to solve this would be by using CSS pseudo classes: button:not(:disabled):hover { ... your code here }App.jsx is an empty array, but the advice slip is an object with properties, such as id. Changing a variable type rarely a good thing, so consider changing the initial state to {} or null.Let me know if you have any questions, I hope my comments are helpful. Keep up the good work and happy coding!
This is my solution for the Profile card component coding challenge
Links
Built with
Any suggestions for improvement are welcomed.
Hey @bhoamikhona,
Nice job finishing this challenge. I really like how close it is to the design and what you did with the hover effect! Also, your HTML and CSS look good. I only have a couple of minor suggestions:
div.pattern-background, which you used for the pattern background and profile image, has a role of img and an aria-label. I don't think you need this. How I see it, the background pattern is purely for visual purposes and has no importance to screen readers or search engines whatsoever, meaning that you can remove the role and aria-label. Besides, the img element inside has a role of img by default, which would make this markup incorrect as images can't contain other images.<p> <span class="stat">80K</span> Followers </p> <p> <span class="stat">803K</span> Likes </p>
Using CSS, you can then lay it out properly, for example with Flexbox.
Hope my comments help you, let me know if you have any questions. Happy coding!
Its good to be here my co-developer.
I find it very difficult identifying the color used and also the size of the project. Any recommendation on tools that could make that easy for me on my next task.
Thank You,
Regards.
Hey @2023course,
Congrats on solving this challenge and welcome to Frontend Mentor! Your solution looks good already, it's close to the design and only has some minor issues.
First, you should be able to get the exact colors from the style-guide.md file that's part of the starter code package. It usually contains all colors, font sizes, and more information, depending on the challenge. You are still left to match the correct color to an element, so you either need to be good at guessing or you can use an editing software like Gimp or Photoshop, which contain a color picker. That's what I would do. Same for dimensions, most editing software contains measuring tools.
Secondly, I've got a few suggestions for your solution:
title to the HTML to make it more accessible and SEO friendly.main element instead of a div.h3 for the title, but an h1 would be more appropriate, as it's the only and therefore most important heading of the page. Headings should always start from the highest level and descend down.div.container, you can apply the same CSS rules to your body and remove the container. One element less ;)@import to load fonts is not considered a best practice, as it's render blocking and could lead to decreased performance. Prefer using the <link .../> code from Google Fonts instead.That's about it. Let me know if you have any questions, otherwise, keep up the good work!
Any Feedback is Welcome!!
Hey @ChrysKoum!
You did a great job on the challenge, I like that the design looks so close. The hover transitions are nicely done. I found a couple of things that you could improve:
main or section. That's also a complaint on your accessibility report. For example, div.card could be replaced by main.card to indicate that this is the page's main content.h5 for the only page title, while an h1 would be the better choice. Each page should have a main title, and then go lower from there. Since "Equilibrium #3429" is the only title, using an h1 would be much better for accessibility and semantics.a or a button for these elements.div elements to center your card. With just a couple lines of CSS, you can make your life much easier:<body>
<main class="card">
Your card content here...
</main>
</body>
body {
display: grid;
place-items: center;
min-height: 100vh;
}
Hope that these tips help you improve the solution. Let me know if you have any questions :-)
Hi
I have completed another challenge using flexbox. While building the project, I found the box shadow and styling the button difficult.
What is the best resource for learning responsive design? Any suggestions to improve this project are welcome.
Hey @mayor-creator!
Nice job on this challenge, it looks good! Don't worry, responsive design is hard to do right in the beginning, you'll get better. If you like learning from videos, I can highly recommend Kevin Powell's YouTube channel, he explains things about web development in a great way, like this one about responsive design.
I went over your code and can give you a couple of tips:
h2. Since it's the only title on the page, it would be better to use an h1. Each website should start with an h1, since it's the most important title describing the page. If there are more headings, you can lower with h2, h3, and so on.button for "Proceed to Checkout". However, you didn't use interactive elements for the remaining two, which is confusing and hurts UX. Even though they are styled as links, clicking on them doesn't do anything. I recommend using a elements, for this challenge they don't need to point to anything.img.music_icon element for the music icon. In this case, I would leave the alt text empty, because it's mostly for screen reader users. They don't gain any benefit from this icon, it's purely for styling purposes and doesn't add anything to the content. So, you could write alt="" to indicate that the image is not important, or even add aria-label="hidden" to hide it from screen readers.Hope these tips help you out, let me know if you have any questions :-)
I await your comments and criticism, do not hesitate, I ask that I improve myself
Hey @mclanecorp,
Congrats on solving the challenge! I like the fact that you used semantic HTML, considered accessibility, and don't have any HTML issues in your solution. Also, it's not far from the design. A couple of things you could improve:
styles.css, but only use 2 of them. @import statements are expensive performance-wise, so it's advisable to use them as sparingly as possible and without loading resources you don't need.div.container, set a max-width and align everything inside the body using the below code:body {
display grid;
place-items: center;
min-height: 100vh;
em or rem, especially for sizing and spacing. These units are more flexible when it comes to zoomed in viewports and scaling your UI based on font size. This video gives a good explanation of choosing the right units.div, this element can be removed.Hope this helps, let me know if you have any questions :)