Any kind of feedback about the project is highly appreciated.
Mihailescu Fanush
@FanushhhAll comments
- @meetarora10Submitted 11 months ago@FanushhhPosted 11 months ago
Hello,
Your solution looks impressive on desktop devices but I believe you still have some work to do on the mobile side of thigs. For that I would recommend avoiding widths and heights on elements and work instead with *max-width" or max-height. These will make sure that your element on which you applied them grows/shrinks along with the device size.
Now onto the javascript side, there is quite a bit of code that can be reduced by selecting all columns as well as prices. I took your solution and made some quick adjustments ( these are by no means the "only way" but more like the way that I found the fastest to showcase my example). I won't really attach the css because it's barebones just to visualize the whole thing.
HTML
<body> <div class="head"> <div class="value"> <p class="name">My balance</p> <h3 class="num">$921.48</h3> </div> <div class="logo"> <img src="logo.svg" alt="" /> </div> </div> <div class="bar"> <h3 class="heading">Spending - Last 7 days</h3> <div class="graph"> <div class="column"> <p class="amount amount1">$17.45</p> <button class="first"></button> </div> <div class="column" style=``> <p class="amount amount2">$34.91</p> <button class="second"></button> </div> <div class="column"> <p class="amount amount3">$52.36</p> <button class="third"></button> </div> <div class="column"> <p class="amount amount4">$31.07</p> <button class="fourth"></button> </div> <div class="column"> <p class="amount amount5">$23.39</p> <button class="fifth"></button> </div> <div class="column"> <p class="amount amount6">$43.28</p> <button class="sixth"></button> </div> <div class="column"> <p class="amount amount7">$25.48</p> <button class="seventh"></button> </div> </div> <div class="week"> <p class="text">mon</p> <p class="text">tue</p> <p class="text">wed</p> <p class="text">thu</p> <p class="text">fri</p> <p class="text">sat</p> <p class="text">sun</p> </div> <div class="line"></div> <div class="next"> <div class="rate"> <p>Total this month</p> <h2>$478.33</h2> </div> <div class="date"> <h4>+2.4%</h4> <p>from last month</p> </div> </div> </div>
Javascript
const buttons = document.querySelectorAll('button'); const columns = document.querySelectorAll('.column'); columns.forEach(column => { const button = column.querySelector('button'); const amount = column.querySelector('.amount'); let columnHeight = amount.innerHTML.substring(1); console.log(columnHeight) column.style.height = `${columnHeight}%`; amount.classList.remove('active'); column.addEventListener('mouseover', () => { amount.classList.add('active'); }) column.addEventListener('mouseout', () => { amount.classList.remove('active'); }) button.addEventListener('click', () => { amount.classList.add('active'); }) })
Here's also a quick codepen containing the css to help you visualize as well: https://codepen.io/Fanushhh/pen/gOqNxGP
Let me know if you have any questions about what's happening and I will answer them to the best of my knowledge. Happy coding!
Marked as helpful0 - @mark-lexSubmitted over 1 year ago
Some problems to resize image for mobile version.
@FanushhhPosted over 1 year agoHey Mark,
Great solution overall, love that you added an animation as well, it's a nice touch.
Your problem mostly appears from setting heights and widths in unnecessary places, for instance, you don't need widths on the box nor the "perfum" container, having it set kills your responsiveness because you're basically communicating to the DOM that your object will have that specific width no matter what.
So first things first, remove any width you got on your children containers and set a max-width and a width of 100%(this one is debatable because if you have elements that have display: block, they will automatically occupy 100% of the container block). Max width works by telling the container that he can occupy 100% of the available space, but if the width of the viewport gets bigger the card itself will limit its width to the specified max-width.
Additionally, if you wanna center things off, maybe you can try adding display flex, justify content center and align items center along with a min-height of 100vh( this is important because the content will only be aligned based on the size of the body, and if your body is as big as your children elements, you won't see any alignment on the vertical axis.
Hope this helps and happy coding, Fanush
Marked as helpful1 - @justinnveraSubmitted over 1 year ago
Hi friends! I had difficulty making the email button responsive. Any way I can improve my code? I'm also new to JavaScript, let me know if I can also improve my code on there.
@FanushhhPosted over 1 year agoHey Justin,
Well done on your challenge! I can see that you only just planned for the layout to be responsive for the specific widths requested in the file, however, it doesn't have to be complicated.
First off, start by removing any fixed width you have on your project, instead, try and work with max-width: (choose your size), width:100%. These two lines of code will make your life easier, if you know where to place them. Why? Because you no longer have to worry about setting the design to be clear for each device size and instead you let the browser do that. When you set the max-width on an element and width of 100%, you're basically saying, I want you to cover the whole width when you have it available but as soon as the width of the viewport gets bigger than the one of the element, you stop at that specific value set on max-width.
I'm not sure if I explained it best but here is a resource to read: https://css-tricks.com/tale-width-max-width/
Best of luck and happy coding, Fanush
0 - @KikeBordaSubmitted over 1 year ago
I would like to improve in the DOM since it was the most complicated thing for me, if you have documentation that could help me it would be of great help.
feedback to my code is also welcome :)
@FanushhhPosted over 1 year agoHello there,
It's really nice how you thought about the whole process and the DOM side seems simple and straightforward, so I don't really have anything to add but very good job and keep going.
1 - @ItachidorriSubmitted over 1 year ago
I faced problem when adding color to the ability titles "Reaction, Memory" etc. Is it because of CSS specificity issues? Help is much appreciated. Hare krsna!
@FanushhhPosted over 1 year agoHello there,
Good job on your solution so far. And to answer your question, yeah, it due to specificity in your CSS. The problem here is that you are specifically setting the headings of the subcontainer flex-2 section to be black which is quite specific in general. I would suggest before setting up a new project to study the design, see which font color dominates the layout and set a general font color on your body tag and then, when you reach a component/section/title whatever that has a different color, set it only for that.
My advice, remove the sub-container-2 h2, h3 colors, add 4 new classes that have the name corresponding the summary items in your html. For instance, summary-item one is red, so created a class called 'red', and add the appropriate properties to change the font color and background color according to the design.
Hope that helps and happy coding! Fanush
Marked as helpful0 - @faruquedewanSubmitted over 1 year ago
I tried completing this challenge using CSS Flexbox but couldn't figure it out. So, learnt CSS Grid and completed this using that. Is it possible using Flexbox? Also, what exactly is the difference between Flexbox and Grid (Didn't even understand the 2D and 3D aspect of them)?
@FanushhhPosted over 1 year agoHello there,
First of all, you did a good job on the challenge so keep it up! :D
The difference between grid and flexbox is mainly that flexbox is used to align items in a single row or column, say a container, it doesn't really handle multiple rows well whereas grid is amazing at that because it has a two dimensional layout(think of it like an excel table while flexbox is more like a word document), you define multiple rows and columns where you wanna place items just like you figured out in this challenge. And to answer your question, yes, I believe it can be done in flexbox as well and what you will also learn on this platform the most is the variety of ways you can do things.
Don't feel bad you couldn't do this challenge with flexbox, in order to grow you have to get out of your comfort zone and experiment on new things. I remember my first time trying flexbox was amazing, the ease of arranging elements on the page, then I had to learn grid which was an imaginary nightmare because I was so comfortable using flex but reality is, some things are flex and some things are grid.
Not sure if my feedback helps, but keep it up, you're doing good, stay hungry!
Marked as helpful0 - @cassialitySubmitted about 2 years ago
All suggestions are welcome!
@FanushhhPosted about 2 years agoHello! Your solutions looks great! I would have some suggestions for the font-family, I haven't tried to import fonts directly into the css file before so I'm not sure how that is working, however it doesn't seem to apply it to your text. Usually when setting a font family you go font-family: 'Lexend Deca', sans-serif; as an alternate, in your code I see you only used the name without the quotation marks so this might be the reason. Additionally, you are repeating some code when it comes to that, setting the family and font-size on different classes. Try setting these properties just inside the body as it will be applied overall and then you can maybe make some changes on individual elements if you wanna tweak something. Also, please search youtube or the resources tab of this website for courses on building responsive layouts as your do not currently have media query for different screen sizes. My recommendation is Kevin Powell( He has a course on this platform about conquering responsize layouts or on youtube as well when there are playlists of videos explaining that.
Cheers and happy coding.
Marked as helpful1 - @M-M-H-RupomSubmitted about 2 years ago
All feedback is welcome
@FanushhhPosted about 2 years agoHello there fellow front-end developer!
First of all I wanna say congrats to you for the solution, it looks good and I want you to keep at it because you're doing great! As a few tips that I've also learned throughout my studies, I would recommend trying to set the background color to the body instead of the main because for the mobile version the background disappears. For the desktop version it's fine because you assigned it to the body which is correct so I assume it was just a simple mistake you added it to the main instead for mobile. Additionally, for the desktop layout things seem a bit off, first thing I noticed is that the text is not left aligned so that's an easy fix. Secondly, as I recently learned from a mistake I did, you might notice the img element does not cover the whole width of the div parent, that's mainly because you need to set all img element to have max-width of 100% and display block, it's part of a general reset of elements that you have to do to get as close to a blank web page as possible. Here's a link that I also got from a friendly developer on the platform that explains it: (https://www.joshwcomeau.com/css/custom-css-reset/) After that , you can maybe select the "last" div class and either make it uppercase to fit more into the original design either by change it in the index.html or you can go in css and add text-transform: uppercase both ways work, it's just about preference I guess.
Cheers!
0 - @LokeshSharma22Submitted about 2 years ago@FanushhhPosted about 2 years ago
It looks pretty amazing so far but it lacks responsiveness when it comes to mobile sizes, which in the challenge is 375px for mobile. You should try creating a media query and adjust the width of the container accordingly. For the background image, I would suggest adding a background-position: center for desktop view. There is also an insignificant error in spelling "Cancel" but other than that, great job!
0 - @AlexandruStefanGherhesSubmitted about 2 years ago
This program went by smoothly, the only problem seemed to be with GitHub pages not displaying my images but I solved it by using Vercel instead.
@FanushhhPosted about 2 years agoEverything looks amazing! Responsive, neat, love the animation of the cube. Did you do a course on SASS? I would love to get some insights on where to get started. I also want to learn about pseudo-classes because I can't understand what's happening there.
0 - @Shinzo99Submitted over 2 years ago
Hello there 🖐,
Not sure if anyone will see this but if u do I'd love some feedback
take care 👊.
@FanushhhPosted over 2 years agoHey Khaled, looks pretty nice but there are some minor changes, such as the image that from the looks of it doesn't have the same size. Your div has 461 in width while your img has 450 causing a small space to occur in the bottom left. You should be able to fix that with a simple adjustment to the width. Additionally, you are also using a different, smaller font weight for the title and a different font family for the discounted price. Also, your github solution link is not working
Marked as helpful0 - @vluboshSubmitted over 2 years ago
pardon for css, main goal was to achieve JS
@FanushhhPosted over 2 years agoAmazing work, Lubomir! One thing that I might add is to add an option if the user doesn't select any stars, either set a condition for the submit button to not work if the user doesn't submit any rating, or simply display "you selected 0 stars" and perhaps a different text where it asks the user to refresh the page in order to rate again or something like that.
Marked as helpful0