Youness• 160
@younessbarahou
Submitted
What specific areas of your project would you like help with?
Is it the shortest way to add these fonctionnalities to the website with js?
P
Clifford Anderson
@cdanderson76
All comments
- PClifford Anderson• 180
@cdanderson76
Posted
Nice work on the project, and I like what you did with the color there. I started off this project the same as you, and in the process, I learned a couple of things:
They always say write 'DRY' code: 'don't repeat yourself'. So while this works fine, there's a consistent repetition of the functions that you have there. Also, with extra '.html' file, there is an additional request sent to the 'server' to grab that page as well, which could cause performance issues .
You could use string template literals to make this work, and write out your code in JavaScript. Check out my code here: https://github.com/cdanderson76/interactiveRatingComponent/blob/main/script.js
See how it looks like HTML? That's what renders the second page. I hope that this helps!
Marked as helpful
1 What specific areas of your project would you like help with?
I would like some feedback on how to improve accessibility.
PClifford Anderson• 180@cdanderson76
Posted
I like this a lot! Has a nice twist to it with the animations!
1- irene• 120
@irene-panis
Submitted
What are you most proud of, and what would you do differently next time?
most proud of utilizing min, a css function i haven't really used before
What challenges did you encounter, and how did you overcome them?
putting anchor tags within list items while making the whole list item clickable - had to look this up. also read up on min() because i wanted the card to be slightly wider but be able to shrink on mobile.
What specific areas of your project would you like help with?
i styled the list items to look like buttons then put anchor tags inside them w/ display block and height/width 100% to make them clickable; im pretty sure this is better than using buttons but lmk if i'm wrong and also if the usage of the list was necessary at all
PClifford Anderson• 180@cdanderson76
Posted
Looking good! I'm not so sure about whether or not you should use lists to crank out those links. One thing you can't go wrong doing, is right-clicking on the page of your app, select 'Inspect', and select 'Lighthouse' (it's at the top of the dev tools menu where you find 'Elements', 'Console', 'Recorder', etc.). Once you've clicked on Lighthouse, it will give you an option to click on a button and analyze your page for accessibility. If something is weird with the way that you've written the HTML, it's sure to give you a clue on what to change! :) NOTE: this is in the Chrome browser...
Also, in reference to using anchor tags as opposed to the buttons, you've laid it out fantastic. If you are just clicking on something to navigate to another part of a page, or a different page for that matter, then you should use an anchor tag. On the other hand, if you click on something, and it does something like renders things to the page, initiates a calculation, etc., then you should use a button! Hope this helps! Great job on the project!
0 - bilal• 30
@M0BLACK
Submitted
What are you most proud of, and what would you do differently next time?
learning by doing is the best way to enhance you skill
What challenges did you encounter, and how did you overcome them?
so easy :) , i did not face any challenge yet
What specific areas of your project would you like help with?
if any one has any advise, welcome. and thanks in advance :)
PClifford Anderson• 180@cdanderson76
Posted
Nice work! There's not a lot that I can say about this one. I would say that when it comes to Github, when you are using file paths, leave the initial '/' off of the file path. For some reason, Github doesn't read those well. You have them in lines 3 and 8 in your 'style.css' file. The font face lines import properly in VS code, but they don't show on Github unfortunately. I changed the code to the way it should be uploaded to your repository:
`@font-face { font-family: "Figtree"; src: url("assets/fonts/Figtree-VariableFont_wght.ttf"); }
@font-face { font-family: "FigtreeItalic"; src: url("assets/fonts/Figtree-Italic-VariableFont_wght.ttf"); }`
I took off the initial '/' in the 'src: url' lines. That way, those files will import properly on Github :) Outside of that, everything is great!
Marked as helpful
1 @Leonardo21042006
Submitted
What are you most proud of, and what would you do differently next time?
--organize HTML better
What challenges did you encounter, and how did you overcome them?
-nothing
What specific areas of your project would you like help with?
-HTML
PClifford Anderson• 180@cdanderson76
Posted
Awesome-looking project! I would say you are definitely on the right track with your HTML, I would just say try doing more with less. Again, this is looks amazing, but you have an excessive amount of 'divs' happening there. Check out my HTML for this project here to see what I mean:
https://github.com/cdanderson76/blogPreviewCard/blob/main/index.html
Other than that, keep up the good work!
1- Beatriz Manrique• 60
@smavend
Submitted
What are you most proud of, and what would you do differently next time?
I'm proud of the fast guideline I could do in HTML, also about clarity with styles in CSS. Different from my first submmit, I checked better the Figma file, making easy to figure out separations and number parameters.
What challenges did you encounter, and how did you overcome them?
At the beggining, I encountered problems with font import, but after some research, I could make it work. For this project I also tried to improve my coding environment, so I searched for some information about formats and so on. Finally, I was able to make some configurations to facilitate next projects coming.
What specific areas of your project would you like help with?
Again, I would like to receive advices about CSS code and what is the better way to structure this file.
PClifford Anderson• 180@cdanderson76
Posted
Great project! I wish I could give you pointers on the way that you've organized your CSS, but it looks pretty good to me from where I'm standing. I will say this, it took me a while to find the CSS file because you have a number of directories/folders that don't SEEM necessary. With the project being a small one, you could store any images in a directory, but the HTML, CSS, and the font that you are importing could all be in the same directory. Again, nothing really wrong with that, it would just take some additional navigating for someone to try and review your code...taking more time. The CSS itself looks great to me though! Nice work!
0 - Andres Castro• 40
@wilmerandres25
Submitted
What are you most proud of, and what would you do differently next time?
Nothing at all. I used the same CSS tools like Flexbox
What challenges did you encounter, and how did you overcome them?
Nothing at the moment.
What specific areas of your project would you like help with?
Accessibility. Sometime a forget to apply it the every web site that I create.
PClifford Anderson• 180@cdanderson76
Posted
I don't know how much closer to this solution you can get. Nice work!
As far as accessibility goes, there's one thing that I would point out. Looks like you're grabbing that avatar image by way of your 'style.css' file (line 100)...which is ok. But, this puts you in an accessibility disadvantage because you can't use the typical 'alt' attribute that you usually have in the HTML doc. What you CAN do is this:
<div class="author-avatar" role="img" aria-label></div>This would replace line 23 in 'index.html'. Either way it goes, nice work on this project. It looks awesome!
Marked as helpful
0 @Sunilk2001
Submitted
PClifford Anderson• 180@cdanderson76
Posted
Hello! Looks like this may have been a minor oversite, but in order to render the image, make sure you get the file path to that image corrected. Use the following, as this is the path when I downloaded your code on my computer:
<img src="qr.png" alt="QR code that redirects to the Frontend Mentor Site">Also, don't forget to add the alt text to images. 😎 This provides assistance for those using screen readers, and boosts accessibility.
1- CompetentKoala• 30
@CompetentKoala
Submitted
I couldn't for the life of me figure out how to get the boxes to stack on top of each other in mobile display.
I know I am doing something wrong with placement of items inside the grid/flexboxes. If anyone knows how i could do this better let me know :)
it looks good. just isn't good for mobile.
PClifford Anderson• 180@cdanderson76
Posted
I agree with the previous comment. This project looks really good, but I'd just add the media query in CSS and use the 'flexbox-direction' property with the attribute of 'column (https://developer.mozilla.org/en-US/docs/Web/CSS/flex-direction). Afterward, adjust margins and gaps as needed. I had this same issue, but with the desktop layout, so I had to use the media query and 'flex-direction' property with the attribute of 'row'. You can find good information on media queries here: https://www.w3schools.com/css/css_rwd_mediaqueries.asp
Happy coding :)
0
