I'm proud of making the app work well with TypeScript, it's something I'm new at. I think I will work more on this project and add Storybook as well.
Florian Stăncioiu
@florianstancioiuAll comments
- @florianstancioiuSubmitted 7 months agoWhat are you most proud of, and what would you do differently next time?
- @florianstancioiuSubmitted about 1 year ago
Hi there,
This is my first time using Tailwind, I still don't think it's a great option but it felt ok using it. For some reason, when I click on the generate new advice button there doesn't seem to be a new HTTP request being made, but when the developer tools are open, the click does trigger the request. It's a little weird, I haven't spent too much time debugging this. Some people told me that it's working perfectly fine. If the issue does appear on your side as well, let me know in the comments so that I can investigate further and fix it.
Florian
@florianstancioiuPosted about 1 year agoI updated my solution so that a new advice would load on mouse click.
The reason why the click seemed to not trigger a HTTP request was because of HTTP caching, I had to cache bust every fetch request. The click was triggering a request, except the data from that request was always the same because of caching...
0 - @ElenaLaura366Submitted over 1 year ago
I am not sure about the image hover effect. I couldn't make it fit the image and let it be a bit bigger at the bottom. I appreciate any feedback about my code like the way I write code or the way others understand the code.
@florianstancioiuPosted over 1 year agoHi Laura,
Here is my advice on the eye section of the challenge:
You can overwrite the existing
.overlay
div with this HTML structure:<div class="overlay-wrapper"> <div class="overlay"></div> <img class="icon" src="" alt="Eye Icon" /> </div>
Don't keep the eye icon inside the
.overlay
div because the opacity property affects every single child element. Use the z-index property to get the.icon
in front of the.overlay
.Note: The z-index property only works if there's a position property on the element as well.
Here is some advice that will help you write better CSS code overall:
-
Install prettier in Visual Studio Code (I asume you use vscode). Follow a youtube tutorial if you can't get the hang of it with the written text, prettier is a must have for a beginner.
-
Use
rem
instead ofpx
, here is a video about it. And here is how to use rem in a nutshell: you set the font-size of the html tag to 62.5% which makes1rem
equal to10px
for all child elements
html { font-size: 62.5%; } body { font-size: 1.4rem; } /* =14px */ h1 { font-size: 2.4rem; } /* =24px */
- Instead of doing this in CSS:
* { margin: 0; padding: 0; box-sizing: border-box; }
Use a modern CSS reset like this one here.
Marked as helpful0 -
- @Jo-cloud85Submitted over 1 year ago
Submitted my 30th challenge! This is one of my major design overhaul attempts in Frontend Mentor. While it is far from a working full-stack e-commerce app, I really spent quite an amount of effort to make the product page look as close as possible to commercial standards - with a header, footer, etc. There are also some additional elements like the review section.
The design inspiration, pictures, and even written content are taken from various existing commercial websites like Nike, Under Armour, Skullcandy, and Unsplash.
Meanwhile, also marks one of my last challenges using vanilla JS. I realize a vanilla JS-based code gets more complex and inefficient to maintain as the project grows bigger and more complex. So, I am learning React and backend stuff and hopefully, come back with better stuff.
@florianstancioiuPosted over 1 year agoWell done, the new design looks fantastic!
Great work, I'm speechless!
2 - @muubaraqSubmitted over 1 year ago
These are the two main issues that I'm facing. Will be glad to get a pointer as to what I'm doing wrong: Updating the total cost price- I get a nan on the total cost price of the product Deleting a product from the cart - the remove button doesn't seem to be triggered. Was working fine before
@florianstancioiuPosted over 1 year agoHi Mubaraq,
Here are some tips to make your code more readable.
-
First off, install prettier for vscode (I'm assuming you use Visual Studio Code, if not, please install prettier for your text editor of choice). https://marketplace.visualstudio.com/items?itemName=esbenp.prettier-vscode
-
Secondly, try to separate the JavaScript logic in different files (You can create a
slider.js
for the slider and acart.js
file for the cart), writting all JS code inside one giant file is a big mistake and should be avoided. -
Thirdly, writting HTML strings directly to DOM is prone to security exploits like XSS. You can aleviate this by using
<template></template>
tags or by using a template engine. Here are some resources on using the template tag: https://javascript.info/template-element , https://www.w3schools.com/tags/tag_template.asp
(Don't lose too much sleep over the third tip)
Now let's address your issues:
- Updating the total cost price- I get a nan on the total cost price of the product
The reason why you get
NaN
is because the string starts with a$
, and that turns intoNaN
. The simplest solution is to remove the$
character usingproductPrice.substring(1)
.// If the product is not in the cart, add it as a new cart item const cartItem = { // ... totalPrice: Number(productPrice.substring(1)) * Number(productCount), };
- Deleting a product from the cart - the remove button doesn't seem to be triggered. Was working fine before
The reason why it didn't work is because the click event is targeting the image element, and the image element doesnt have the
.remove-button
class, so we have to look for the nearest/closest element with that class.To fix it, replace this line:
if (event.target.classList.contains('remove-button')) { // ... }
with
if (event.target.closest('.remove-button')) { // ... // And comment this function call bellow, otherwise you will get an error: // updateCartTotalPrice() }
As a last note, don't create functions inside event handlers, create the functions in the global scope. Also, don't create event listeners inside event listeners.
// BAD addToCartButton.addEventListener('click', () => { function updateCartItemCount() {} function updateCartItem(cartItem) {} function updateCartTotalPrice() {} cartIcon.addEventListener('click', () => {}); });
// GOOD addToCartButton.addEventListener('click', () => {}); function updateCartItemCount() {} function updateCartItem(cartItem) {} function updateCartTotalPrice() {} cartIcon.addEventListener('click', () => {});
I think you will still face some issues after tweaking the code using my advice, especially with the cart. Please don't take my advice the wrong way, I'm only trying to help you out. I hope this will help you a bit in your journey, keep on coding!
Florian
Marked as helpful2 -
- @florianstancioiuSubmitted over 1 year ago
Hi there,
I wanted to write some CSS and HTML for fun and this is the result. I know that the testimonials section should've been a slider but implementing it would've taken more time than I planned to spend on this challenge.
The one thing I learned while working on this project is that the background image CSS property can take multiple images followed by commas.
I plan on revisiting this project and adding scroll animations.
Florian
@florianstancioiuPosted over 1 year agoHi there,
As I mentioned in the solution description, I revisited the challenge and updated the github repo with scroll triggered animations. I followed David Omotayo's article on LogRocket.com about Framer Motion. In that article, he suggest using a different library for detecting if an element is in viewport, I didn't went down that path, I realized that Framer Motion has it's own built-in
useInView
hook that does the same thing as the library he suggested, and in the end, I used only Framer Motion.That's all, Florian
1 - @Ripra87Submitted almost 2 years ago
Thank you guys to check my really first project with js! I have a question if somebody would check my work, about the Javascript file. In the index.js i save the selected vote (1 or 2 or etc) with addEventListener, and a function that store the value in a new variable to bring it in the second html page.. well, i was wondering if it was possible to do it without the eventListener, if there is another way to store a variable from a selected button after a click. This way is really short, but even if i used it i don't really understand it (i copied from a project a did during the js course). I mean, for me the addEventListener should be used to add an event, such as play a sound, show an image, text, or something that happen (the event) after the click, but in this case i used it only to take out a value and put it inside a variable, is there a more "proper" way to do it? Thanks everybody for the suggestions and happy coding!!
@florianstancioiuPosted almost 2 years agoHi Ric,
You shouldn't have 2 index files, and localstorage shouldn't be used in this project. You can take a look at how I did the challenge here: https://www.youtube.com/watch?v=oZJb_GPAlc4 The JavaScript part is done at the end of the video, you can also find the code in the description of the video.
Marked as helpful0 - @HicksznSubmitted almost 2 years ago
I'm used to adding a link for the font-family with a preconnect to the head section of my index.html but I recently saw that you can just add an import URL to the top of the CSS file. Am I understanding that correctly and is there a best practice?
I'm also used to using html { font-size: 62.5%;} but someone recently told me not to do that. Can anyone explain why? I learned from Mosh Hamedani that it makes calculating the font size easier because this way 1 rem = 10 px.
@florianstancioiuPosted almost 2 years agoHi Zana,
I personally like to use the <link> tag in the head section to request the fonts, I don't like my CSS file to trigger any requests instead I want to see them all in one place, in HTML, I think it's more intuitive this way, but there isn't a best practice regarding this, you can do as you like.
As for rem, I also use
html { font-size: 62.5%; }
, I recently discovered it on my own (even though I have years of development behind me...). I think it's an awesome way of creating UIs and making them accessible while retaining developer friendliness. Where did you saw that rem shouldn't be used? I want to learn more about this too.Kindly, Florian
Marked as helpful0 - @florianstancioiuSubmitted over 2 years ago@florianstancioiuPosted over 2 years ago
Apparently, I forgot to add that pesky warning sign when there's an error on an input. :(
0 - @willettoSubmitted over 2 years ago
Please no copy/paste comments. :)
I would love to know a better way of adding the rating stars without pasting the <img> tag 15 times.
What do you think was the most elegant way to offset/stagger the ratings and purple cards? I ended up using translateX and Y.
@florianstancioiuPosted over 2 years agoHi,
I don't think there's a better way of adding the stars (there is another way of adding the images that would not involve writing them multiple times but the click functionality would be lost and without it, the ratings are useless).
As for the ratings, I used
position: relative
and set theleft
property to a negative value. The ratings could have been done also using a negative margin but I think the way you did it with translate is probably the most elegant way.Keep on coding!
Marked as helpful1 - @NickODxyzSubmitted over 2 years ago
Hi all,
Background images and how to move them around absolutely kills me. It would be great if I can get some tips on how best to do this. The two on this little project were a nightmare lol
What is the go to way for getting a background image to appear? This time round I used padding but it was confusing and I had to use a random % number for it to work. Any tips would be great on this one.
If you could let me know if I went he best way about adding them in to the HTML and then positioning them in the CSS, that would be great.
Thanks, Nick
@florianstancioiuPosted over 2 years agoHi Nick,
You can actually use
background-image
with multiple images on the body or the containing div you have and after you do that you can position each individual image usingbackground-position
. Make sure to disable the repeating backgrounds usingbackground-repeat: no-repeat;
. You don't need to have 2 extra divs for the background images, it might work that way though but I haven't tried it.1