Thise challeng was the firt time I create a project using JSON data by my self and without any help so it take me a long time and of course javascript code is repeated many time
What specific areas of your project would you like help with?How can I improve my JS code when I use JSON data
Hellooo :), very good solution for this challenge, Ilham Bouktir. 🍳
I have a few recommendations that can help you in your future projects:
General:
my-project/ ├── build/ │ ├── assets/ # Static files (images, fonts, etc.) │ │ ├── img/ │ │ ├── fonts/ │ ├── css/ │ │ └── styles.css │ ├── js/ │ │ ├── main.js │ └── index.html │ ├── src/ │ ├── assets/ │ │ ├── img/ │ │ └── fonts/ │ ├── scss/ │ │ └── styles.scss │ ├── js/ │ │ ├── app.js │ │ ├── components/ │ │ │ └── Header.js │ │ └── utils.js ├── .gitignore ├── README.md
HTML:
<article class="products__card card"> <figure class="card__figure"> <picture class="card__picture"> <source media="(max-width:600px)" srcset="" /> <source media="(max-width:610px)" srcset="" /> <img src="" alt="Waffle" class="card__img" /> </picture> <figcaption class="card__figcaption"> <span class="card__name">Waffle</span> <h2 class="card__details">Waffle with Berries</h2> <span class="card__price">$6.50</span> </figcaption> </figure> <button class="card__btn"> <span class="card__addBtn">Add to Cart</span> </button> </article>
This would really improve your SEO. If I can give you some advice, when you're given designs, try to section them by "boxes" or "blocks" that contain other "boxes." In this case, each dish represents independent information, so I wrap it in an <article> element, which makes the browser understand that this is an independent component representing a piece of information (with its own description, image, name, and price). This could be within a <section>, and you can continue experimenting with this approach. It will give you more practice.
CSS:
The responsive design seems to be incorrectly applied :(. You could opt for structuring the dish cards using display: grid, with grid-template-columns: repeat(auto-fit, minmax($size, 1fr));, and you will have more flexibility in how you apply the responsive layout.
I would also recommend a class methodology, like BEM, to avoid specificity issues in your CSS.
JS:
function getcarts() {
fetch('data.json')
.then(response => response.json())
.then(data => {
let cartsCount = data.length;
createCarts(data, cartsCount);
let btnContainers = document.querySelectorAll(".cart .image");
addToOrder(btnContainers, data);
})
.catch(error => console.error('Error loading carts:', error));
}
(for example, in createCarts, addToOrder). This causes a lot of calculations and changes to the DOM, which can negatively impact performance, especially if you have a large number of elements. Use DOM fragments (document.createDocumentFragment()) to create elements in memory and then insert them all at once at the end.function createCarts(obj, count) {
const fragment = document.createDocumentFragment();
for (let i = 0; i < count; i++) {
let cart = document.createElement("div");
cart.classList.add("cart");
cart.id = i;
fragment.appendChild(cart);
}
contnt.appendChild(fragment);
}
(notAddBtn, addedBtn) have a similar creation pattern. You're repeating a lot of code that could be simplified. You could create a generic function for creating buttons that accepts parameters to customize them.function createButton(className, iconClass, textContent) {
let btn = document.createElement("button");
btn.classList.add(className);
let icon = document.createElement("i");
icon.classList.add(...iconClass.split(" "));
btn.appendChild(icon);
if (textContent) {
let text = document.createTextNode(textContent);
btn.appendChild(text);
}
return btn;
}
let notAddBtn = createButton("notAdd", "fa-solid fa-cart-plus", "Add to Cart");
let addedBtn = createButton("added", "fa-solid fa-plus");
General Recommendations:
I hope these recommendations were helpful. Keep it up, and if you ever have any questions or get stuck on something, feel free to reach out to me on LinkedIn.
Happy coding! 🍧
I learnt a lot of few things I'm going to apply from now on.
What challenges did you encounter, and how did you overcome them?Media query can be tough. Building mobile first and improving css knowledge can improve responsiveness. Work to do.
What specific areas of your project would you like help with?Building mobile first.
Hello 👀, very good solution for this challenge, Jeancy.🍕
I have a few recommendations that can help you in your future projects:
Having a solid code structure is key when starting any project.
assets folder to store images, icons, and fonts.Downloading the fonts directly would improve the page’s performance, as it avoids unnecessary domain resolutions. Additionally, in CSS you can use the @font-face rule to load fonts locally.
It would be better to contain the .social links within a <nav> instead of a <div>. This improves semantics.
Consider adding an aria-expanded to the share button to indicate whether the share menu (div.social) is visible or not, if it's interactive.
alt: Image descriptions should be more specific when necessary. For example, instead of alt="facebook icon", something like alt="Facebook logo" or a description more relevant to the context of use.
Instead of using a <div> inside the <footer>, it would be more appropriate to use a <section> or a <div> with role="contentinfo" for the section containing the author information.
Consider using the <figure> element to wrap the article image, as it's content related to the text and has better semantics.
Repetition of the value 14px in styles like .warning, .small, .footer. You also repeat values in the media queries, like :.photo img { position: relative; }.
Reduce styles for tags you don’t use in your project, as this also helps you decide how your reset will be and eventually create your own reset for specific projects.
I noticed the design breaks at 425px, and I believe it’s due to excessive unnecessary positioning. For example, .content-text should not be absolute. The element that should be absolutely positioned is .social, because it's this element that we don’t want to interfere with the design flow.
In the media queries, only change the minimal necessary styles. For example, if you set .card { display: flex; }, it’s not necessary to define .card { display: flex; } again unless you want to change the display. In general, minimize the styles.
social variable was being obtained inside the click event, which caused the element not to be found on the first click, and the action wouldn't execute correctly. This happens because the social element wasn't available until the click event was fired, making the if condition never true.The solution is to move the declaration of the social variable outside the event, so it’s available from the start, before the click happens.
Additionally, instead of modifying the style directly with social.style.display, it’s better to use CSS classes to manage the visibility of the element. Use classList.toggle('visible') to toggle the visibility of the social container using classes, which is more flexible and easier to maintain.
I hope these recommendations were helpful. Keep it up, and if you ever have any questions or get stuck on something, feel free to reach out to me on LinkedIn.
Happy coding! 🍧
Hello
I haven't completed a solution in a long time so I'm fairly happy with how this went. Took awhile but I got the hang of the newer sass syntax with @use and @forward instead of using @import.
Fonts didn't work - at first
To begin with none of my fonts were loading and I couldn't figure out why. After some research I found out that it was because I was using url's relative to my sass partial, and not to where the partial code is resolved. It wasn't a bad thing though as I learnt something new and got to experiment with optimizing fonts.
What specific areas of your project would you like help with?Article, div or li?
I used an h3 in place of h2 or h1 as I viewed the component as part of a larger real life page, but that's not what confused me.
I wasn't sure whether the main card should be an article, div or li. The card could easily be part of a carousel of li's, it could also be a stand alone div or article, this is where I was confused. In the end I decided to make it an article.
Any advice on what tags are most appropriate and what tags I could use for .product__content and .product__thumbnail would be appreciated. These feel like dividing sections/containers to me, and I wasn't sure whether a div or section would be most appropriate.
Thanks for reading, open to any advice/tips!
Hello 👀, very good solution for this challenge, Carl.
I have a few recommendations that may help you with your future challenges:
Having a structured codebase is key when starting any project.
Use an assets folder to store images, icons, and fonts.
Regarding the structure of the SCSS, of course, this is very personal and depends on how you get used to developing, but it would be good to centralize your partial imports in one file.
Since having a _base.scss partial and importing @use './components/index'; that in turn imports @forward './cards' is unnecessary.
You can improve this by having a central_file.scss, and directly importing the necessary partials into it, and placing the mixin and variable files that will be used by the partials there.
Replace the div with <figure> and <figcaption> to improve the semantics and accessibility when working with visual content related to text.
Add aria-label to buttons to improve the accessibility of interactive elements.
If you want to see the component in a broader context, like a perfume store, we have a section for perfumes, and each perfume is an independent piece that forms part of the same section, so the use of article is correct. :)
Remove unused styles and simplify the CSS/SCSS code, as in the reset I see you have reset.scss with styles for tags you don't use.
Centralize all styles in one main file (e.g., styles.scss) to avoid creating unnecessary extra files.
Use @forward only for sharing mixins, functions, and variables, not for component styles, like you do with _card.scss (you can import this directly).
As for mixins, you can really use them if multiple styles repeat, or if multiple structures repeat, like layouting with flex or grid.
I hope these recommendations have helped you, keep it up, and if you ever have a doubt or get stuck on something, feel free to message me on LinkedIn.
Happy coding 🍧
Hi 👀, very good solution for this challenge, Obare!
I have some recommendations that may help you improve even further in your future challenges:
HTML:
SEO Improvements: Ensure that the <h1> tag is unique for each page. Add a description to the alt attribute of your images to enhance accessibility.
Semantic Tags: Use more semantic tags like <main>, <section>, and <article> for each card. Use the <figure> element for images along with a <figcaption> to describe the content.
Button Accessibility: Add an aria-label to buttons to improve accessibility for screen readers.
Styles:
React:
I hope these recommendations have been helpful!
Happy coding 🍧
I just completed another JS beginner challenge from Frontendmentor. Please share your suggestions to improve. Thank you.
Hi 👀, very good solution for this challenge, Aizirek.
I have some recommendations, which may help you in your next challenges:
HTML:
<img class="item-heading-img" src="./assets/images/icon-star.svg" alt>
JS:
I hope this recommendations have helped you, keep it up, and if at any time you have a doubt or you get stuck with something, write to linkedin.
Happy coding 🍧
Hey! 👀, very good solution for this challenge Jose Alfredo. 🍕
I have a recommendation for you, I see how you don't use Js, and you handle it with Css, that's good but sometimes it's not so flexible.
For example when I leave one detail open and open another one the card expands and its size changes every time I open another detail, so that breaks the design a little bit.
You can use the @font-face-rule instead of importing fonts, I leave you a very interesting resource about this: @font-face-rule
Use relative measures such as <em>, <rm>,<ch>, for better flexibility.
I hope this recommendations have helped you, keep it up, and if at any time you have a doubt or you get stuck with something, write to linkedin.
Happy coding 🍧
Hi B4rka🎡, good solution for this challenge.
Answering your question and checking your <img> tag the src attribute of the tag you forgot to put "./" which means from the current directory to reference the img , because if you put "/" you would be referencing from the root.
<img src="./assets/images/avatar-jessica.jpeg" class="img" alt>
Some recommendations that I could give you in HTML:
When working with images try to put the alt attribute, it serves for accessibility and gives you a better semantic for your page.
As for the use of the <article> you could opt for another more generic tag, since <article> is a self-contained tag, and has its own value.
Also the use of semantic tags, such as <nav> for your links, use of a <section> for the letter container or an <article>, this so that you have more semantic value.
CSS
If you have a question, or have conflicts with a project you can write me on linkedin. 🎈
Happy coding.🎗
Hey!🎃 Benji, good solution for this challenge.
First of all, these situations are normal in any learning environment. ✨
Some recommendations I could give you in JS:
Repetition of selections: In your code you select the inputs, this results in an unnecessary search every time the form is submitted, you could select the inputs outside the event.
Cascading classes: You are adding and removing the error class directly to the parent elements (input.parentElement), it works but it would be better if you use it directly in the input.
Redundant Mail Validation: You don't need to add and remove the error class twice for mail validation, you can remove the second case from the if handling.
I can think of something like this, more or less:
const isValid = isEmailInput ? validateEmail(input.value) : input.value.trim() !== "";
if (!isValid) {
input.classList.add("error");
} else {
input.classList.remove("error");
}
If you have any questions, feel free to contact me. 🎨🥼
Happy coding.🎞
Hey! 👀, very good solution for this challenge G-DAMS. 🎃
Beyond simply defining its visual appearance, semantic tags provide meaning for search engines, screen readers. As the tags you used in this project are semantic and themselves mean and structure the most important parts of the web for better accessibility.
I can leave you some resources: Semantic HTML Accessibility
And a suggestion in your CSS styles:
You can directly use flex or grid to center your boxes, without needing to use absolute positions which can be a bit of a headache.
And in your links that had trouble with the GitHub link, you can use column classes and add a modification class. I base this on the BEM methodology, as you can reduce the call you make for each tag body #container ul .special-item{}, as it has a lot of specificity and is less flexible. With BEM, or another methodology, not necessarily BEM, you can better structure your classes and HTML files, avoid problems with specificity, and duplication of styles.
I hope these comments help you for your next challenges, keep it up, being constant is the key 🥇.
Happy coding 🎉.
Hey!👀 , very good solution for this challenge Luigi. 🎃
I have some recommendations that may help you in future challenges.🧨
HTML:
<img src="icon-star.svg" alt aria-hidden=true>
CSS:
As for the colors, you can really change them, or put your characteristic touch in the projects, as long as they are related to the page.
Also you have some problems when it comes to height and aesthetics with the buttons, you can try giving a "border-radius:50%" and "height:4rem" and play with this, so that it does not break and looks proporcinal.
Also the common styles can be grouped in classes, to avoid duplication of styles.
I hope these comments help you for your next challenges, keep it up, being constant is the key 🥇.
Happy coding 🎉
Ey!👀 , very good solution for this challenge josrubfer. 🎇
I have some recommendations that may help you in future challenges.🧨
HTML:
In images you should always have the alt="" attribute, which is itself an alternative text, the alt attribute will make sure that the engines do not miss important sections of the pages.
There is no need for a tag <figure>, or <figcaption> you could include the tag for the name, but it would be a bit forced.
The use of <buttons> inside the <a> , is not correct, you could just use the a tags, since you are actually directing to another page, and a <button> fulfills a specific functionality within the page in most contexts.
You could choose to learn some Methodology of classes for your tags like BEM , I leave you the link if you are interested: Methodology BEM
CSS:
The pseudo-class of ":active" would be ":hover", not ":active", already with the passage of the cursor is more intuitive and we interpret faster that it is an interactive tag
Use of relative tags like em, rem , to handle padding, margin and fonts, there are more you can see it here: Relative Units
I hope these tips help you and keep going.
Happy coding 🥏
Hi OliCB , very good solution for this challenge. 🎊
I have some recommendations that may help you in future challenges.
HTML:
Starting every project with an <h1>, although it makes use of a <header>, does not have the same meaning as an <h1>.
When we find ourselves with images that are only decorative, do not have a strong impact, then we can choose to use an aria-hidden. in the decorative images:
<img class="card__icon" src="assets/images/icon-star.svg" alt aria-hidden="True" />
Use of semantic tags, <article> to represent in itself a self-contained or complete element, for example it would be better to use the <article> to encompass an entire side of the letter or better a section, and you could separate it into two sections Front and back section.
The use of forms a direct interaction with the user and where they send some kind of data, it would be better that you replace the <div> by <form> and use <inputs> of type radio buttons, since you will receive data, and instead of <div> as button, replace it directly with the <button> tag so that the HTML code has a value by itself and is not something generic, but something semantic in which search engines and screen readers have references and know where they are.
Also, although it may seem more complicated to structure in this way, at the time of making interactions with Js, it is much easier and more flexible.
I hope these tips help you and keep going.
Happy coding 🥏
Hi Emmanuel Akpan very good solution for this challenge.🎇
I have some recommendations that I hope will help you.🎨
HTML:
CSS:
Regarding the hierarchy of headings, while going from a h1 > h4 , depends on the context and the importance of each section, it can be considered less semantic than following a sequence one by one (h1>h2>h3>...h6), Since search engines are based on a hierarchy of headings intended to structure the content in a logical and coherent way. 🎇
Use of relative measures such as (em, rem, ch, &) for the boxes, as they help the flexibility of the project and adapt automatically in some cases.
Use of some methodology, as you can increase the specificity of the styles:
main .sectiona_third {
display: flex;
flex-direction: column;
gap: 10px;
}
main .sectiona_third h2 {
color: var(--Nutmeg);
font-size: 28px;
}
The second one has more specificity and more time to set styles, it would be better something like:
.main__seciton {
display: flex;
flex-direction: column;
gap: 10px;
}
.section__h2 {
color: var(--Nutmeg);
font-size: 28px;
They have the same weight of specificity, and show what they refer to.
main .sectiona_second {
display: flex;
flex-direction: column;
gap: 10px;
}
main .sectiona_third {
display: flex;
flex-direction: column;
gap: 10px;
}
main .sectiona_second ul {
display: flex;
flex-direction: column;
gap: 10px;
/* margin-left: 20px; */
list-style-position: outside;
padding-left: 25px;
}
main .sectiona_third ol {
display: flex;
flex-direction: column;
gap: 10px;
list-style-position: outside;
padding-left: 25px;
}
Css Methodology or a structure for the elements you could eliminate the duplicity of your styles:
.main__section {
display: flex;
flex-direction: column;
gap: 10px;
}
.section__list {
display: flex;
flex-direction: column;
gap: 10px;
/* margin-left: 20px; */
list-style-position: outside;
padding-left: 25px;
}
I hope you will find these tips useful for next projects, overall very good code and project. 🎑
Happy coding. 🧨
Hi Daniel Kull very nice project for this challenge.! 🥞
I have some recommendations for it 🔎 , hope they will be useful for you for next ones:
Html:
<h2 class="subtitle--prep-time">Preparation time</h3>.CSS :
You could opt for some use of Class Methodology for your tags like BEM as there are some stylos that repeat, this ensures minimum specificity.🎈
Separate the reset css in other files and also the fonts, it would be something more modular for your project. 🎲
Overall a very good project, happy coding.🎨
Hi momorocks111, first of all there is nothing wrong with watching tutorials and it is part of the process to feel uncertainty with some projects.
This is not technical but it is advice (it is my thought and may or may not help you):
Is it bad to do it, is it bad to follow tutorials? :
When you get stuck with a project (like this one) instead of watching tutorials on how to do the whole component it wouldn't be bad, but you wouldn't actively learn, and in programming it's active practice/theory.
It would be better to focus on the concrete problems and thus complement your learning and learn to search for information on the internet for the things/modules you need to review.
But well I have no context if you have enough time to learn comfortably or if your situation is good to just focus on this, but don't be discouraged enjoy this is the process and take it easy. 🍜
Hi Muhammadjavohir nice contribution to this project.
I have a small recommendation to make to you as I have not been able to see your code in depth.
I imagine that you use a set of classes to activate/deactivate the accordions, but I see that you allow us to open all the headers and this can break your design and make me have to close them all manually and it's a bit tedious :(
Then you could make clicking on one of the headers activate the class only on that one and on the others disable them, it's a nicer action and you avoid problems with your design🥓
Happy coding 🍔
Hi cmdarcy very nice project for this challenge.
Answering your question , as to whether there is an easier way to position the error icons in the input tags , with the use of ::before and ::affter you can also achieve this:
.container {
position: relative;
}
.container::after {
content: url("icon-error.svg");
position: absolute
top: 0;
right: 0;
}
Another valid option is to set the error icon as background.
But these options depend on the specific needs you have and how you want to structure the html and css.
Happy coding 🥨
Hi Soph-iee very good project for this challenge, I see that you had complications in your JavaScript. Before I check and help you to remove the duplication in your code.
I want to see your HTML structure:
<h1></h1> in the html is understandable as it does not have an impact with the design , but semantically it is , and in terms of accessibility and SEO.(While it is a practice , it is good to keep this in mind).-The use of semantic tags such as a <main></main> for card/component, is better structurally, as they have more semantic meaning than a .<div>.
<img src="images/illustration-thank-you.svg" alt="thank you" /> it is not necessary to use the atl, you could use an aria-hidden:true, as they are just decorative.The section with which the user interacts:
<div class="rating-word">
Please let us know how we did with your support request. All feedback is
appreciated to help us improve our offering!
</div>
<button class="ratings one unclicked">1</button>
<button class="ratings two unclicked">2</button>
<button class="ratings three unclicked">3</button>
<button class="ratings four unclicked">4</button>
<button class="ratings five unclicked">5</button>
<button type="button" class="submit-btn">Submit</button>
</div>
The use of buttons or inputs, may vary according to the project, but for this one it is more convenient to use inputs instead of buttons.
Buttons can be beneficial when you need them:
You want to allow users to select multiple options from the 5 available, rather than a single option.
Each option has an immediate action associated with it, and you don't want to wait for the user to click a submit button after each selection.
But that's not what we're looking for with this project, so it would be better to use radio type inputs. why?:
So looking at these points the structure could look like this :
<form action="#" class="card__form">
<fieldset class="card__options">
<legend class="card__title hidden">rate us from 1-5:</legend>
<input
class="card__check"
type="radio"
name="options"
id="option-1" />
<label class="card__option" for="option-1"> 1 </label>
<input
class="card__check"
type="radio"
name="options"
id="option-2" />
<label class="card__option" for="option-2"> 2 </label>
<input
class="card__check"
type="radio"
name="options"
id="option-3" />
<label class="card__option" for="option-3"> 3 </label>
<input
class="card__check"
type="radio"
name="options"
id="option-4" />
<label class="card__option" for="option-4"> 4 </label>
<input
class="card__check"
type="radio"
name="options"
id="option-5" />
<label class="card__option" for="option-5"> 5 </label>
</fieldset>
<button class="card__button" type="submit">SUBMIT</button>
</form>
Css:
It is better to integrate in your styles the use of relative measures (rem, em, %) instead of absolute ones like (px).
You could use some methodology to apply styles and improve the way of naming classes and consequently the structure of your Css styles (You could start with the use of BEM)
If you are interested : Bem Methodology
JavaScript: One thing I can notice is that you focused a lot more on the code itself, but if you look closely, simply using the Html structure radically simplifies the javaScript code.
Variable declaration:
const SubmitButton = document.querySelector('.submit-btn');
const oneStar = document.querySelector('.one');
const twoStar = document.querySelector('.two');
const threeStar = document.querySelector('.three');
const fourStar = document.querySelector('.four');
const fiveStar= document.querySelector('.five');
const rated= document.querySelector ('.selected-number');
const ratingPage = document.querySelector ('.rating-div');
const thankYouPage = document.querySelector ('.thank-you-page');
You can abstract the logic, for example: What happens when a user clicks on btn 1? and on btn 2...? We take a number and add it to the back card.
You can use a querySelectorAll('options') to select the inputs since you have the same functionality.
So if you have the same functionality we should not create a function for each button, just a function for the 5 inputs, that will be activated when we click on the submit button.
Look at this is the simplified code:
const $card = document.querySelector(".main__wrapper");
const $button = document.querySelector(".card__button");
const $rating = document.querySelector(".card__rating");
const $options = document.querySelectorAll(".card__check");
function setRating() {
let selectedOption = Array.from($options).find(option => option.checked);
let message = `You selected ${selectedOption.nextElementSibling.textContent} out of 5`;
$rating.textContent = message;
}
function hangle(event) {
event.preventDefault();
$card.classList.add("main__back");
}
$button.addEventListener("click", hangle);
$options.forEach(option => option.addEventListener("change", setRating));
Obviously this is only one of many solutions that exist, you can modify or use any part of the code that interests you, no answer is absolute and no code is perfect.
I hope this comment is useful not only for this project but for the following ones, although it will take you more time to take all these recommendations, it is better (It is my opinion and it can change).
Greetings and enjoy the process. 🥞