@RubenSmnSubmitted over 1 year ago
I had some difficulties when trying to add the hover effect for the image. I managed to get it right by using position: absolute. I am wondering is this a valid solution or should this be avoided?
Michael Waaler
@10highAll comments
- @10highPosted over 1 year ago
Hey Ruben - nice job. Yeah, position: absolute is valid here (in my humble opinion).
I see you've used Astro. How are you liking it? And can I ask why you're using it, rather than vanilla HTML and CSS? (just curious).
1 - @codezelossSubmitted about 2 years ago
Hi everyone! I just completed another challenge using Reactjs (My first Reactjs ⚛️ project).
Please let me know about any issues you may find and how I can improve my solution especially my code. I'm always open for your feedback (;.
Have a nice day!!
@10highPosted about 2 years agoHey - I don't know React yet, so I can't comment on your code, but I found myself on Github and everything checked out :)
Didn't see any issues! Well done.
1 - @itsmusaSubmitted about 2 years ago
I would be happy to hear your feedback!
@10highPosted about 2 years agoHey - it looks great. Good job.
My only feedback would be that you're not using an semantic markup for accessibility. You've done pretty much all of it with <div> tags. Accessibility is important for people using, e.g., screenreaders, to navigate a page.
Think about checking it out, because it's becoming increasingly important. Otherwise, great work.
0 - @ElektrokatSubmitted about 2 years ago
Hello guys, please I need a code review on this, I did not really have issues with this, but could there be a better way to do it? Thank you in anticipation.
@10highPosted about 2 years agoHi Edwin - nice job.
The only thing I would comment on is that your media query on the <main> tag sets the size to be same between width 375px and 1440px. As you can see from the comparison above, this means the content remains the same size for both mobile and desktop and is therefore not responsive. Ideally, you want the smaller version to appear for mobile and the larger version to appear for desktop.
Also: you use absolute positioning to center everything in the screen. A "better" approach is to make your <body> tag:
min-height: 100vh; display: flex; justify-content: center; align-items: center;That will achieve the same effect, plus it would also center your footer, which is looking a little lost in the corner of the screen :)
Marked as helpful0 - @10highSubmitted over 2 years ago
While I'm happy with the result that you see on screen, I'm unhappy with the underlying "code". I think there could have been a much simpler and cleaner way to achieve the same result. I feel that my solution is too "hacky", relying too much on margins and padding to achieve a perfect match against the original.
The main thing I learned/realized during the process is that I need to do much more forward-planning (now that these challenges are becoming larger and more complex). I think I also need to see the Figma design, so that I can work out the dimensions in advance and (hopefully) inform my approach to the solution.
I'd love to hear from anyone on their experiences of having access to the Figma design files!
I'm feeling more confident using semantic markup, but it's such a big subject and tips and pointers would be most welcome! :)
Thanks for taking a look. Have a great day!
@10highPosted over 2 years agoI fixed the errors created by the Font Awesome "fonts" for the social media icons by using the SVGs also available from Font Awesome and removing the JS script import from the top of the HTML file. (You can see it commented out in the index file.)
I also added aria-labels to the SVGs nested inside links, which seems to have satisfied the lack of discernible text errors I had previously.
Now no errors, yay!
0 - @10highSubmitted over 2 years ago
While I'm happy with the result that you see on screen, I'm unhappy with the underlying "code". I think there could have been a much simpler and cleaner way to achieve the same result. I feel that my solution is too "hacky", relying too much on margins and padding to achieve a perfect match against the original.
The main thing I learned/realized during the process is that I need to do much more forward-planning (now that these challenges are becoming larger and more complex). I think I also need to see the Figma design, so that I can work out the dimensions in advance and (hopefully) inform my approach to the solution.
I'd love to hear from anyone on their experiences of having access to the Figma design files!
I'm feeling more confident using semantic markup, but it's such a big subject and tips and pointers would be most welcome! :)
Thanks for taking a look. Have a great day!
@10highPosted over 2 years agoHm, the report has generated a bunch of errors, most of them seem to be from using Awesome Fonts for the social media icons. If anyone has any experience with this and how I can resolve them, please let me know.
Thanks. :)
0 - @nicolas055Submitted over 2 years ago
Hello everyone! I'd like to know how can i improve my writed code? What I did wrong or how can i do it with less lines?
@10highPosted over 2 years agoHey there, your solution looks really good, but as you can see from the generated report, your code has a number of issues.
I think it would be a good idea for you to start looking into accessibility, such as using semantic markup. Here's a good intro to the subject: https://www.w3schools.com/accessibility/
Good luck and keep up the good work.
Marked as helpful1 - @NiezzxSubmitted over 2 years ago
There are still some small problems, when I resize the screen, the button position is not parallel because of the difference in content length; and the button itself has changed in size because of the reduction in size. I hope someone can give me some suggestions.
@10highPosted over 2 years agoI originally had the same problem. I solved it by making each card its own grid and using grid gap to space the segments. Looking back this was not the best way, but you can set the height on each grid row and then the cards will always be the same size.
Good luck!
Marked as helpful0 - @SupafeiSubmitted over 2 years ago
I made it with flexbox... Can't put it on the center of the page. And it's seem to be an error with loading images
@10highPosted over 2 years agoTo center your flexbox, you need to put it inside another flexbox and justify-content and align-items to center. You should set the min-height of your body to 100vh.
Have a look at my solution to see how: https://www.frontendmentor.io/solutions/responsive-3column-card-html-and-css-7R3-nlUMp
Hope that helps.
0 - @catherineisonlineSubmitted over 2 years ago
Hello, Frontend Mentor community! This is my solution to the Intro component with the sign-up form.
I appreciate all the feedback you left that helped me to improve this project. Due to the fact that I published this project very long ago, I am no longer updating it and changing its status to Public Archive on my Github.
You are free to download or use the code for reference in your projects, but I no longer update it or accept any feedback.
Thank you
@10highPosted over 2 years agoSorry, I'm not good enough yet to offer feedback, but it looks so great. How do you (and others I've noticed) get the dimensions to match so well? I'm spending far too long comparing the images against my browser. I feel there's a method I don't understand yet. Any enlightenment? :)
0
