Open to any feedback on how to improve my code.
Ibrahim San
@ibra-sanAll comments
- @victoriatuadySubmitted over 1 year ago@ibra-sanPosted over 1 year ago
Hey! Congratulations on completing your challenge and well done! Awesome work seriously!
I checked your code and your preview and quite honestly there is nothing to add, unless if I have to be very nit picky. So just a tip if you want to make your finished product pixel perfect is to increase your spacing between the text, and slightly increasing the size of your header (perfume name). Spacing in web design is important, add more whitespace as much as content, this will make your work look more elegant. Use this tip for future projects as well. Balance the whitespace with content, make the content swim in whitespace, but not to the point that is drowning, just swimming.
Anyways, awesome work and keep up the great work. Looking forward for your future work and all the best in your future endeavors.
0 - @prjwl16Submitted over 1 year ago
I did not have the Figma file, so I wont be pixel perfect but I tired.
@ibra-sanPosted over 1 year agoHey awesome work and congratulations on completing the challenge. Quite honestly you have used some awesome technologies for this project super awesome.
If you allow me to be a bit picky here there is something you might have missed out on (which happens a lot when you are coding) and that is the hover effect over the bars. Once the cursor goes over the histogram bars its suppose to show the expenditure of that specific day. Other than this little small thing, you work is definitely picture perfect.
Again congratulations on you finishing this project, and all the best in your future endeavors.
0 - @mariotbg11Submitted almost 2 years ago@ibra-sanPosted almost 2 years ago
Hi, awesome work mate. This is incredible actually when I used the slider on the page in order to see the difference by the live and the design, its almost identical. You are talented mate.
- Just a tiny little advice (maybe you didn't notice it, or you might have left it out), the page is not changing view as the screen gets smaller. The image and how the page is structured should change as the view gets smaller.
But most definitely, that is seriously some awesome work. Keep it up
0 - @MahidunNobiSubmitted almost 2 years ago@ibra-sanPosted almost 2 years ago
Hi, awesome work you did their mate. Seriously awesome work, and there aren't any major issues at all, in fact that are no issues period. Though I will just be a little be nit picky.
- I love how the divider stays fixed from xl sized screen all the way to lg or md screens; however once it goes from the md to sm screens or lower, the divider starts off big then goes to small. My humble advice is to fix the divider's length and width as you have done for screens with bigger view ports.
Again love your work right there, and that is just me being nit picky. Its actually better than the solution I have provided haha.
0 - @reallyUndefinedSubmitted about 2 years ago
Hey everyone,
Had some fun with challenge.
As always feedbacks and suggestion are welcome. 😊
@ibra-sanPosted about 2 years agoCongrates on completing the challenge! Superb work!
A small suggestion would be making the color of the bar changes depending on the day to show the total expenditure in that day. For example if today is Friday, then the Friday bar should be in Cyan the rest should be in orange.
Other than that, excellent work and keep on coding!
Marked as helpful0 - @JoelLHSubmitted about 2 years ago
This one was a tricky for me, had to google a lot. If theres any way i can improve it pls let me know. Thank u
@ibra-sanPosted about 2 years agoHey! Congrates on completing the projects. Superb job!
Haha I need to take some few points from you myself. If would allow me to be a bit nit picky, then this would be my only suggestion.
- The bar should be green color depending on the day of the week, not on the cursor click. For instance if today is Monday then once the page is loaded the bar representing today should be colored green indicating the expenditure of today. This could be achieved by getting which day of the week is it using the date function in JS. See if todays day matches the names of the day in your HTML. If it does then change the background to green, else leave it as normal.
This suggestion is after serious nit picking. Congrates man on a superb job and have a nice day.
Marked as helpful1 - @cacostedSubmitted about 2 years ago
Hello this challenge was really interesting and I would like to know how could improve my solution, any advice or suggestions are welcome.
@ibra-sanPosted about 2 years agoHey! Congrates on completing the project mate. To be quite honest, you have done a super job on this one. I know because I have done worse on this one haha.
Just a small suggestion here is to take a look at the menu arrow button (Features and Company preciously). If I click on the text of those menu items the drop down appears and disappears, which is awesome. The issue is when I click on the arrows, the drop menu doesn't appear or disappear. A fast fix to this one is to just make the drop down arrow and menu name in one container, and throw and event listener onto the entire component; therefore, no matter the user clicks the drop menu will appear and disappear. The second suggestion is to make a menu disappear or close when another one opens. So let us say if I click on the company menu while the features menu is open, the features menu should close and the focus should only be on the newly opened company menu. This could be achieved by creating an event listener that listens to clicks or events happening on the nav bar. Check if the click happened on a dropdown menu item. If the click happened on a dropdown menu item then give that item a special class name, let us say opened. Once you click a item it should be closed by removing that special class name. The addition of that class name should be associated / targeted with the CSS styling created for the drop down menu. Meaning once the click happens on that item, then the dropdown menu and its display, features and styling should appear, that is controlled by the whether the class name is added to the tag element or not. In the CSS target the class name is already targeted and the styling is added to it. The JS controls whether this item should have that class name with the styling or not. This is just an overall concept, the details is up to you and you checkout some implementations of this concept on YouTube most definitely.
Haha at this point I am just nit picking man. Overall your work is super, and wish you the best in your career man.
Congrates!
Marked as helpful0 - @hamid-abdallah-mohammedSubmitted over 2 years ago
the responsiveness of mobile css media query for mobile how do I get better
@ibra-sanPosted over 2 years agoCongrats on completing the project.
-> If you mean like changing the image based on the size of the view port. Then there is not need to use a media query for that. Rather add srcset attribute to your image and set it according to your design wishes. Below is a helpful guide around 10 - 20 minutes read depending on how fast you digest the content:
https://developer.mozilla.org/en-US/docs/Learn/HTML/Multimedia_and_embedding/Responsive_images
This will tell you how to use srcset.
-> If you are talking about media queries in general. There is something you need to take in mind, only the styling attributes that belongs to a certain tag mentioned written inside the media query is going to affect the mentioned tag / element when the view port is changed. For example: if I set height to 20px and width to 20px, and in the media query I set width when viewport is 700px to be 10px. Then only the width is going to be affected, the height is going to be the same at 20px. A good resource for media query is the following, also a short read. https://www.w3schools.com/css/css3_mediaqueries.asp
Anyway congrats on finishing the project. Al Salam Alikum
Marked as helpful0 - @lucashdoaSubmitted over 2 years ago
- I'm not sure I applied BEM correctly. What would you change?
- I feel like the ratings number are not fully aligned center in the circle. How would you achieve that?
@ibra-sanPosted over 2 years agoGood job on completing the project. Quite honestly I don't know if I should give you advice or not because you have done better than me in terms of accessibility and html issues.
Well I am not perfect in BEM and I have been struggling with it, but reading this BEM documentation should help. Now this is around 10 -30 minutes read depending on how fast you digest content (Link: https://en.bem.info/methodology/html/). After reading the documentation you will have a very solid understanding of BEM.
The link I sent is for BEM regarding HTML; however, there is BEM regarding JS, there is file structuring, etc. Basically this website will help you organize your projects, hence give them a read as well if you wish.
Bonus tip: Use BEM with Sass / Scss and you will see how quick and organized are your styling flies.
-> When it comes to circles and centering content in them: 1) I create a square. Meaning make sure the width and height are the same size. 2) Border radius the square to its max. Meaning make sure the border radius of the square is a huge number. There is a certain threshold for border radius that if you pass the border radius will stop been bothered. The bigger the number that more sure you will be about passing that threshold. 3) Flex the square (circle after step 2), justify the content to center, and align the items center. Everything inside that circle will be vertically and horizontally aligned.
Anyways congrats on completing the project.
Marked as helpful0 - @adamwinzdesignSubmitted over 2 years ago
I spent much longer than I'd like to admit trying to figure out how to handle the correct positioning for the tooltip dynamically when hovering over a bar element, but I still had fun and I learned a ton about chart-js by working on this project.
@ibra-sanPosted over 2 years agoNever knew there was something called chart.js
Thanks for sharing!
0 - @MyntsuSubmitted over 2 years ago
My only issue was being able to change the font on hover/focus when hovering or focusing on the button, like the example shows.
I thought of a variable that interacts with the
font-size
of the<p>
but I have no clue how to actually pull it off. If anybody knows, I would gladly be advised how to do it! - @KKTTAAIISubmitted over 2 years ago
How do I scale the svg tag? I tried using just width and height but that does not really scale the tag. How do I make the image smaller or bigger?
@ibra-sanPosted over 2 years agoCongrats on completing the challenge.
-
Its awesome that you could actually use the svg tag and make it output something, I actually never knew such a tag even existed, that is really brilliant actual. Since its my first time hearing about it, I can't really help you with the tag. But there is another way...
-
I am sure you downloaded this challenge's file on your PC. Head over to images folder you will see icon-cart.svg. Even though it says svg I imported it as an normal image using an img tag like so : <img src="images/icon-cart.svg" />. Therefore I can edit its size and control it. That is my work around.
That is all.
Congrats, and all the best.
0 -