Andrew G. Ayad• 100
@andrew-g-ayad
Submitted
CharlieeLuna
@CharlieeLuna23
All comments
- CharlieeLuna• 260
@CharlieeLuna23
Posted
Everything looks good, nice job!
If I'm allowed to add a little feedback, I'm noticing that you added an ID to each button and created a variable for each one to later make an array with all those variables. An easier and cleaner, in my opinion, way to do this is to add a class to all the buttons and then you can use something like:
const btnsArray = document.querySelectorAll('.the-classname-you-chose');This will give you an array with all the items that share that class, in this case, your buttons.Hope this helps!
Marked as helpful
1 - Mirm• 80
@Missteeme31
Submitted
I'm having issues utilizing containers to group items for CSS positioning. What is easier to use, flexbox or grid?
CharlieeLuna• 260@CharlieeLuna23
Posted
Hello, mirm, you did a really good job here!
Let me provide you some tips and feedback that has worked for me.
First off, try to clear all of the HTML/Accessibility Issues listed. For example, you don't have a title in your HTML. After the body, usually comes the <main> tag which ensures that your website has the most important information there, in this case, just wrap everything under the that tag.
Regarding your problems with the border-radius on the left side, what's happening is that your card has that but your img doesn't. It happens to me too, it seems that the image is not being affected by that design on your card in CSS so just make sure to apply the same design to the image. You can eitre use something like: "border-radius: 10px (top-left), 0px (top-right), 0px (bottom-right), 10px (bottom-left). Or you can just use the actual selection with top-left-radius: 10px.
Also something you may want to try, remember that this projects are intented to be design in a "mobile-first" view, meaning that you design everything in mobile view and then you upscale to desktop. It's not as hard as it sounds, feel free to check my own solution if you need any hints: https://www.frontendmentor.io/solutions/product-preview-card-R3zhJwDAGo
Hope this helps and if you have any questions, feel free to ask!
Good luck on your learning journey!
Marked as helpful
1 - ADCByDefault• 190
@ADCByDefault
Submitted
!help. i can't have add the hover affect on the social icons. tell me how you would do it. thanks a lot in advance.
CharlieeLuna• 260@CharlieeLuna23
Posted
Hello!
To change the color of the icons you have to use svg images which I know you are but you are adding the image itself. Honestly, I'm not the best person to explain this because I watched a video on SVG's for this and I don't really remember but let me paste you a snippet on how that part looks in my html file:
<a href="#"> <i class="fa-brands fa-facebook fa-2x"></i> </a>That would be one logo and the other two would just change the name to fa-twitter and fa-instagram. And for me to use them I had to download a library.
Once you are able to use Svg's it would be the same as with the button, with the :hover pseudo class.
A quick tip if I may, it could just be an issue with Github, but on your html file you have a lot of indentation and I, personally, found it hard to read. The recommended indentation is 2 spaces afaik.
Marked as helpful
1 - Yu-An• 70
@yuany2036
Submitted
I feel like I struggle a bit with making things responsive. I have so far started off making the desktop version first, and then moving on to the mobile view, and usually have to experiment quite a bit to make everything responsive and presentable. One problem I had for example was the fact that when stacked in the mobile view, elements that are stacked at the top simple get cut off, and there is no way of scrolling up there (due to being in a negative position I presume), which I was able to fix by removing my justify-content: center. However I feel like there has to be a more standard way of making it responsive? Any suggestions on best practices in this case?
CharlieeLuna• 260@CharlieeLuna23
Posted
First of all, good job!
I can personally recommend to start with a "mobile first" approach. I did it that way and when moving to desktop I only needed to change a few things like the border radius and the display direction instead of column, row as everything was already centered. Just 18 extra lines of code for the media.
Hope this helps for future projects.
Marked as helpful
1 @Mosestule2003
Submitted
Sorry the full depiction may not appear identical. I urge you to please reference the github repo, to get the accurate work. Thank you.
CharlieeLuna• 260@CharlieeLuna23
Posted
Nice job! There's room for improvement.
Something that I learned the first time I uploaded a project is how to correctly add images.
In your HTML instead of using:
- <img src="Images/icon-music.svg" alt="" class="music-icon">
You would want to use:
- <img src="./Images/icon-music.svg" alt="" class="music-icon">
Is minor but makes a major difference as "./" means "Current directory" which will use your reposition (or anywhere your files are located) as the current directory and be able to find the correct path.
Also inside the "style-guide.md" you can find the color for the background and don't forget to add the image too.
0@omargtdev
Submitted
Hi, everyone! This is my solution, and I have two questions for you:
Is it ok the way I apply the BEM method in this case? and Is the style ok or can it be better?. For example in the hover of the image, I've put the image within a div container with another div to do that effect.
Thanks.
CharlieeLuna• 260@CharlieeLuna23
Posted
Nice job!
One tip I can provide for centering the container is to use "display: flex;" with a some more styling, that's the solution I used and most people use for this cards projects and it will help in the future. In this situation you only have that card but if you were to add more content the "absolute" would cause a problem I believe.
Marked as helpful
1- JJ Vega• 110
@jjvega86
Submitted
Feedback welcome on the following areas:
- Semantic HTML. Did I structure my HTML well here? How can I improve the ways I chose to write my markup?
- Usage of padding/margin throughout the component. I felt like some of my code here was "hacky" but maybe I'm overthinking it a smidge.
CharlieeLuna• 260@CharlieeLuna23
Posted
Congrats and nice job!
One thing I can mention is that the you should not call the last div in the card "footer" as "footer" as it is actually the attribution at the bottom of the page which should be in the <footer> tag. I used "Creator" for that bit.
Regarding margin, I would not advise to use margin as a tool to center the card. For most of these projects I just used display: flex combined with other elements to center the content.
I'm no expert myself so take this with a grain of salt but it is what I was able to find.
0 - Marina Aix• 60
@aixmarina
Submitted
This is my first challenge so I would appreciate if you could give me some advice on how to structure my code. Also, all kind of improvements that I can add to my solution are welcome. I found it difficult to do a responsive design. That was the hardest part. Thank you so much.
CharlieeLuna• 260@CharlieeLuna23
Posted
Hi! A newbie here so take my advice with a grain of salt :)
You did pretty good for your first challenge, congrats!!
Little tip that I can provide for structure is to not repeat your classes unnecesary. e.g. In your styles sheet you have your hero image as ".card .card-image img" when you could have used only ".card-image" which is the class you assigned to the img.
This repeats several times and I feel it makes it hard to read as there's a lot of repetition.
Also, what is it that you found difficult for the responsive design?
Again, pretty good job!!
Marked as helpful
0
