Design comparison
SolutionDesign
Solution retrospective
need feedback about my code where i can improve myself in minimising lines of code
need feedback abou usage of colors and styling
Community feedback
- @JAleXDesigNPosted about 1 year ago
Hello @tejastej1997, regarding how you can improve your Javascript code:
- Improve Variable Naming: Instead of using generic variable names like boolean, percentage, and btn, consider using more descriptive names that convey their purpose. Use camelCase for variable names to follow the standard JavaScript naming convention.
- Reduce Repetition: You can store repeated elements or class names in variables to avoid repeated querying of the DOM. This can also help if the class names ever need to change in the future.
- Break down your code into smaller functions or methods. This makes the code easier to understand, test, and maintain. Create separate functions for different functionalities like calculating the tip, updating the UI, handling error messages, etc.
- Validation: Add validation to input fields to ensure that users enter valid values. For example, ensure that bill amount and number of people are positive numbers.
Here's an example of how you might apply some of these improvements:
//Uso de "const" para definir variables de elementos seleccionados y cambiar a **camelCase** sus nombres const customBtn = document.getElementById("custom-btn"); const customInput = document.getElementById("custom-input"); const reset = document.getElementById("reset"); //Change the selector of all buttons with tipcalculator__options-opt classes from: ".tipcalculator__options-opt" to ".tipcalculator__options-opt:not(.custom)", this to prevent it from including the "custom" button since it is being accessed this in the constant "customBtn" and will have its own eventListener const tipPercentageButtons = document.querySelectorAll( ".tipcalculator__options-opt:not(.custom)" ); const selectedTip = document.querySelector(".tipcalculator__selected-tip"); const enterButton = document.getElementById("enter-button"); const billInput = document.getElementById("bill-input"); const billAmount = document.getElementById("bill-ammount"); const perPerson = document.getElementById("per-person"); const peopleInput = document.getElementById("people-input"); const numberOfPeople = document.getElementById("number-of-people"); const tipInput = document.getElementById("tip-input"); const tipAmount = document.getElementById("tip-ammount"); const totalBill = document.getElementById("total-bill"); const percentageInput = document.getElementById("percentage-input"); const inputs = document.querySelectorAll("input"); //Add more descriptive variable names let selectedTipValue; let customTipSelected = false; let percentageSelected = false; customBtn.addEventListener("click", () => { customInput.classList.remove("hide"); selectedTip.classList.add("hide"); customTipSelected = true; }); reset.addEventListener("click", () => { selectedTip.classList.add("hide"); perPerson.innerHTML = "$0.00"; totalBill.innerHTML = "$0.00"; inputs.forEach((inputField) => { inputField.value = ""; }); }); function updateTipPercentage(tipValue) { tipAmount.innerHTML = `${tipValue}%`; } //Change selectedTip[0] to selectedTip tipPercentageButtons.forEach((button) => { button.addEventListener("click", () => { selectedTipValue = button.value; updateTipPercentage(selectedTipValue); selectedTip.classList.remove("hide"); customTipSelected = false; percentageSelected = true; customInput.classList.add("hide"); percentageInput.classList.add("hide"); }); }); customInput.addEventListener("keyup", () => { toggleElementVisibility(selectedTip, true); updateTipPercentage(customInput.value); }); //Separate the validation and calculation logic in a separate function, and pass to the click event of the "enterButton" function calculateAndDisplay() { const bill = parseFloat(billAmount.value); const numberOfPeopleValue = parseInt(numberOfPeople.value); if (isNaN(bill) || bill <= 0) { billInput.classList.remove("hide"); billInput.innerHTML = bill === 0 ? "Bill ammount cannot be Zero" : "Bill ammount cannot be empty"; perPerson.innerHTML = "$0.00"; } if (isNaN(numberOfPeopleValue) || numberOfPeopleValue <= 0) { peopleInput.classList.remove("hide"); peopleInput.innerHTML = numberOfPeopleValue === 0 ? "Cannot be zero" : "Cannot be empty"; return; } let tipPercentage = customTipSelected ? parseFloat(customInput.value) : parseFloat(selectedTipValue); if (isNaN(tipPercentage) || tipPercentage < 0) { percentageInput.classList.remove("hide"); billInput.classList.add("hide"); return; } peopleInput.classList.add("hide"); const perPersonAmount = (bill * tipPercentage) / 100 / numberOfPeopleValue; perPerson.innerHTML = perPersonAmount.toFixed(2); totalBill.innerHTML = (perPersonAmount * numberOfPeopleValue).toFixed(2); } enterButton.addEventListener("click", calculateAndDisplay);
Regarding the css code:
- Only import the necessary fonts:
For this project you only use "Space Mono", so it is not necessary to import the rest of the fonts using
@font-face
just import:
@font-face { font-family: "Space Mono"; font-style: normal; font-weight: 700; font-display: swap; src: url(https://fonts.gstatic.com/s/spacemono/v13/i7dMIFZifjKcF5UAWdDRaPpZYFI.ttf) format("truetype"); }
- You can add the "font-family" to the "body" and so you would not have to add it to each class of each element:
body { font-family: "Space Mono", monospace; }
- Consolidate Repeated Styles: If you have repeated styles for certain elements, consider consolidating them into a single CSS class. This can help reduce redundancy and make your code more maintainable.
- Use CSS Variables for Colors: Instead of hardcoding colors directly into your CSS, consider using CSS variables for colors. This makes it easier to change color schemes throughout your application.
:root { --main-bg-color: #c5e4e7; --primary-color: #00494d; --secondary-color: #5e7a7d; --highlight-color: #26c0ab; --text-color: #00494d; }
- Group Media Queries: Group media queries that apply to the same element. This can help reduce duplicated code and make media queries more organized.
I hope my recommendations help you. Good luck!
0@tejastej1997Posted about 1 year ago@JAleXDesigN thanks for your valuable time you put to give the feedback will update according to your suggestion
0
Please log in to post a comment
Log in with GitHubJoin our Discord community
Join thousands of Frontend Mentor community members taking the challenges, sharing resources, helping each other, and chatting about all things front-end!
Join our Discord