I would like to get a comprehensive feedback on the cleanliness of the code, readability, usability, efficiency, and so on. Perhaps somewhere I should have used a different method of solving the problem. That's what I'd like to know.
Hi good work,
. Your HTML is well structured.
. For your CSS, I recommend centering your card. You can do this by using:
.container {
display: flex;
justify-content: center;
align-items: center;
height: 100vh;
}
Good continuation
I'm proud of building a mobile-friendly navigation menu with a hamburger icon and mastering Flexbox and Grid for a responsive layout. I used React's useState hook for menu state management, ensuring a dynamic experience. For the future, I plan to implement a slider for the photo gallery, focusing on state management to control the active image and adding navigation buttons for a smoother user experience.
What challenges did you encounter, and how did you overcome them?One aspect I spent some time on was refining the positioning of elements using Flexbox and Grid. I experimented with various layout configurations, adjusting properties like display, justify-content, align-items etc. I also utilised browser developer tools to test responsiveness across different screen sizes, ensuring a consistent and adaptable design.
Hi,
I like your React components, they are clean and have well-structured JSX.
I have some advice for you :
You can keep each component in its own folder and include its CSS file there. This makes it easier to reuse the components.
Consider placing your images and other assets (like font) in the public folder.
It’s a good practice to use the map to display elements in React :
const PhotoGallery = () => {
const images = [
{ src: MilkBottles, alt: "milk-bottles" },
{ src: Orange, alt: "orange-on-table" },
{ src: Cone, alt: "ice-cream-cone" },
{ src: SugarCubes, alt: "sugar-cubes" },
];
return (
<section className="gallery-section">
<div className="gallery-grid-container">
{images.map((image, index) => (
<img key={index} src={image.src} alt={image.alt} />
))}
</div>
</section>
);
};
export default PhotoGallery;
const Testimonials = () => {
// 1. Define an array of testimonials
const testimonialsData = [
{
image: Emily,
alt: "Emily",
review: "We put our trust in Sunnyside and they delivered, making sure our needs were met and deadlines were always hit.",
name: "Emily R.",
occupation: "Marketing Director",
},
{
image: Thomas,
alt: "Thomas",
review: "Sunnyside’s enthusiasm coupled with their keen interest in our brand’s success made it a satisfying and enjoyable experience.",
name: "Thomas S.",
occupation: "Chief Operating Officer",
},
{
image: Jennie,
alt: "Jennie",
review: "Incredible end result! Our sales increased over 400% when we worked with Sunnyside. Highly recommended!",
name: "Jennie F.",
occupation: "Business Owner",
},
];
return (
<section className="testimonials-section">
<h1 className="testimonials-heading">Client Testimonials</h1>
<div className="testimonials-grid-container">
{testimonialsData.map(({ image, alt, review, name, occupation }, index) => (
<div key={index}>
<img src={image} alt={alt} />
<p className="review">{review}</p>
<p className="name">{name}</p>
<p className="occupation">{occupation}</p>
</div>
))}
</div>
</section>
);
};
export default Testimonials;
Good continuation
i deploy project on local instead of upload build folder to github
this project make me learn about how to handle background-image and learn to use react react-router-dom to changing page to sent information between route
What challenges did you encounter, and how did you overcome them?this project is not hard but i use 5 days to finish because i am so busy maybe if i'm not lazy i can finish this in one day
What specific areas of your project would you like help with?feel free to comments my code i think is not clean and too complex for other to edit after me , if it's group project i think it's hard to handle please comment thank you.
Hi, good work!
I have a few suggestions for you:
Try creating more components; your two current components are too large. For example, you could have a drag-and-drop component, an input component, and a button component, all of which can be grouped together in a form component. You can also do a TextWrapper component for the text at the top.
It’s a good practice to keep each component in its own folde with its own CSS file. This approach makes your components easier to reuse in other projects.
Good continuation
Est ce que c'est assez ressemblant ? qu'auriez vous fait de différent pour améliorer ce code ?
Bonjour,
Ca ressemble bien à l'original.
Quelques conseilles pour l'html :
Pas besoin de span ici : <p><span>Learning</span></p>
Ici tes titre doivent ce suivre au lieu de mettre H2 et H6 tu dois mettre :
<h1>HTML & CSS foundations</h1>
<h2>Greg Hooper</h2>
Pour le css
Ici si tu veux centrer l'élément en utilisant flex tu dois écrire display : flex pas block
.blog-card {
display: block;
justify-content: center;
align-items: center;
height: 550px;
width: 320px;
margin: 20px;
}
Bonne continution
I would like a true feedback about the structure, the code, the approach, anything that would help me to evolve. Thank you!
Hi, good work
I have some suggestions to improve your code:
here an exemple :
/**
* IconDelete component
* @param {React.SVGProps<SVGSVGElement>} props - Props for the IconDelete component
* @returns {React.ReactElement} The IconDelete component
*/
const IconDelete: React.FC<React.SVGProps<SVGSVGElement>> = (props) => {
return (
<svg
width={props.width || 22}
height={props.height || 22}
viewBox="0 0 25 25"
fill="none"
xmlns="http://www.w3.org/2000/svg"
className={props.className}
{...props}
>
<path
d="M6.25 19.7917C6.25 20.3442 6.4695 20.8741 6.8602 21.2648C7.2509 21.6555 7.7808 21.875 8.33334 21.875H16.6667C17.2192 21.875 17.7491 21.6555 18.1398 21.2648C18.5305 20.8741 18.75 20.3442 18.75 19.7917V7.29167H6.25V19.7917ZM8.33334 9.375H16.6667V19.7917H8.33334V9.375ZM16.1458 4.16667L15.1042 3.125H9.89584L8.85417 4.16667H5.20834V6.25H19.7917V4.16667H16.1458Z"
fill={props.fill || 'currentColor'}
/>
</svg>
);
};
export default IconDelete;
Then, you can use it in your JSX like this:
<IconDelete width="30px" height="30px" fill="red" />
I finally completed this app taking long time.
What challenges did you encounter, and how did you overcome them?there are lots of challenges, first was to use react for first time.
What specific areas of your project would you like help with?making it more responsive and get idea on how to make more components to make it more reliable.
Hi, good work,
To help you make your design responsive with Tailwind CSS, you can use breakpoints like "sm" (for mobile) in your SelectionElement component.
You can also use "sm" to change your title font-size.
Tailwind CSS is a mobile-first solution, so you can write w-[90%] for the mobile view and sm:w-[546px] so that everything above the breakpoint will be 546px:
<div className="sm:w-[564px] w-[90%] flex-shrink flex flex-col gap-6">
I advise you to look into the "tailwind-merge" and "clsx" libraries; they will help you clean up your JSX.
I also advise you to check "eslint-plugin-tailwindcss." It's a game-changer because it automatically organizes your Tailwind classes when you save your file. (but it's not yet working for Tailwind CSS 4.)
Good learning!
I'm proud of how I used Grid and Flexbox to make the layout responsive, and how I implemented JavaScript to manipulate DOM classes for the share button. Next time, I'd focus on improving the structure and efficiency of my code.
What challenges did you encounter, and how did you overcome them?Designing the share button for tablet and desktop views was challenging, especially adjusting its behaviour based on screen size. But it wasn’t too difficult, it was a new concept for me, and I actually found it fun to figure out!
What specific areas of your project would you like help with?I'd love to get feedback on my JavaScript—specifically, where I can improve and what good programming practices I can implement in the future.
Hi good work,
For your code, I would advise you not to handle responsive in JavaScript. Just use CSS. (Even though doing it in training is good for trying new things)
. For do simple you can just do a toggle and hide the popup with css :
const btn = document.querySelector(".article__share");
btn.addEventListener("click", () => {
popup.classList.toggle("hidden");
});
in css :
.hide {
display: none;
}
. Also, do not put your CSS file in the 'asset' folder. It's better to create a 'style' folder directly in the 'article-preview-component' directory.
Good continuation
I was proud to have designed this myself, while fairly quick I think it still gives it a bit more personality.
What challenges did you encounter, and how did you overcome them?I struggled trying to remember mobile-first with my media queries and with the absolute positioning, especially in tablet sizes.
What specific areas of your project would you like help with?If there is a better way to set absolute positioning, I would absolutely love to know. or an alternative that would work in this circumstance.
I think you can put the position : relative to the card so your image will follow the card when you do responsive
I tried to write semantic and well-structured code, focusing on organization, legibility, and accessibility. I'm using these challenges to research better ways to do things, one point at a time.
What challenges did you encounter, and how did you overcome them?Perhaps the main focus here is to ensure the correct image appears at the appropriate size. I addressed this by using media queries, alternating between display: none and display: block.
I would appreciate feedback on code structure, the use of semantic tags, accessibility, the application of BEM methodology, or any obvious areas where I might have complicated things.
Hi, good work,
. For accesibility you can write different alt for every image. For exemple alt="chanel parfum mobile" and alt="chanel parfum desktop" alt="shopping cart"
. For semantic maybe its will be better to write something like this :
<body>
<header>
</header>
<main>
<section>
</section>
</main>
<footer>
</footer>
</body>
I am proud that I was able to complete it successfully.
What challenges did you encounter, and how did you overcome them?The main challenge I encountered was positioning the profile image correctly. I resolved this by experimenting with different CSS properties and adjusting the padding and positioning to achieve the desired alignment.
What specific areas of your project would you like help with?I would love feedback on how to make the design match the given design more accurately, especially the background. Any tips or suggestions on achieving the right look would be greatly appreciated!
Hi, good work! I suggest using a combination of relative and absolute for the two circles. For example:
Its should help maintain the correct alignment of the circles.
This is the first challenge where I did not get access to the Figma file so I was doing the CSS all via looking at the image and creating the code from scratch.
What challenges did you encounter, and how did you overcome them?Adjusting the padding and margin. Getting used to using rem as measurement for the different objects.
What specific areas of your project would you like help with?I am getting better at CSS. I am looking more at organising my codes better.
Hi, nice work You can center your element by setting height: 100vh on the body. I also suggest you to adding a transition to your hover for a smoother animation and applying cursor: pointer.
body {
height: 100vh;
}
.button {
transition: background-color 0.3s ease-in-out, color 0.3s ease-in-out;
cursor: pointer;
}
Similarity to original, I guess
What specific areas of your project would you like help with?How can I simplify my code while adhering to best practices?
Hi, good work. The only minor thing I noticed in your HTML is that you don't really need to include a <div> before the <section>. I wish you the best of luck with your learning
I am most proud of getting the validation to work, & am excited to get more used to doing that.
What specific areas of your project would you like help with?As usual, anything I could improve on, do simpler, etc. Thank you!
Hi, good work. However, you don't need to add these lines in your JavaScript :
function onload() { //happens when page is loaded
dismissBlock.classList.add('hidden');
errorMsg.classList.add('hidden');
}
onload();
if you set the element to be hidden by default in your HTML. For example :<div id="success-container" class="hidden"> Then, once the validation is successful, you can simply remove the hidden class via your javaScript to display the element (same for the error message).
Centering the buttons without align-items: center. Align-items:center kept resizing the buttons to their minimal width, so I ended up using margin:auto instead to center them.
Would love an explanation as to how that is? Or if my solution is the "correct" one...
What specific areas of your project would you like help with?Good luck for your study
Hi, I like the idea of green icons when the form is complete.
For the password you can add a type="password" in your input, like this when the
user will write you will have stars ******
<input type="password" autocomplete="off" id="password" placeholder="Password">
Good continuation.
Hi, Nice work. I like the alert with name of user when we register.
Maybe you can try to use the function "submit" it's will be more realistic than on click :
form.addEventListener("submit", (e) => { e.preventDefault(); } });
In your Html maybe you don't need to use an ul for the form.
Good continuation
Hi, Nice shad effect! I don't knew it before. It will be great if you add an effect or text when we validate an email.
I try my email and I have an error message. I think your Regex don't read majuscule.
Good continuation!
Hi, Nice work, I think it's will be good if you can leave the popup when you click again in the button. With a toggle or on click. Good continuation.
