@DicksenT
Submitted
@Theosaurus-Rex
@DicksenT
Submitted
@Theosaurus-Rex
Posted
Hey Dicksen, sorry it took me so long to getting around to leaving feedback - thanks for your patience!
Here's a couple of things I found that could help improve this project a bit:
<h1>
for the logo, but you should use the image provided in the images
folder with the template download<input>
as a child of a <ul>
element - you can only have <li>
elements inside a <ul>
radio
inputs for the percentage amounts, rather than just list items.text-transform: uppercase;
. This helps screen readers and other assistive technology parse your page better.Otherwise, this is a big improvement compared to your previous challenges, nicely done!
@WillsX9
Submitted
Hopped on my initial challenge after a few weeks in front-end development. I'm eager to elevate my skills through participation in Frontend Mentor Challenges and contribute to the community. I value and welcome your feedback and suggestions!
@Theosaurus-Rex
Posted
Hey @WillsX9 , congrats on completing your first challenge! π
There's a few things that can be tweaked here to improve the overall structure of your markup:
header
tag is not appropriate for this use case. Typically this tag is used for things like navigation in a web page. In this case, all of the content is a single group, and all of it can be wrapped in the main
tag.<br>
in the middle of a line of text. Instead, let the text wrap within its container - this will make your content work better across a range of screen sizesYour styling overall looks really good! I think you may have just missed the text colours but otherwise the design looks spot on π
Marked as helpful
@leanghok120
Submitted
@Theosaurus-Rex
Posted
Hey, this looks awesome!
A couple of small suggestions:
text-transform: uppercase
. This helps assistive technologies such as screen readers to have an easier time parsing the page content.Other than that, awesome work!! Really solid underlying structure and the final result looks great! β¨
Marked as helpful
@dev0xgenius
Submitted
@Theosaurus-Rex
Posted
Hey there! Congrats on completing the challenge!
A couple of small pieces of feedback:
section
tags are particularly necessary here, but if you do want to use them I would be wrapping everything in them rather than just your text - I'd probably have the icon, all of the text and the "Learn More" CTA inside a section each.button
for the "Learn More" CTAs. These are likely to be links to another part of a website, not buttons that a user is using to make a decision. Here is a great article to learn more about this.img
tags all need to have the alt
attribute to be valid. In this case, you can set the icons to have alt=""
, as they are purely decorative. Here is a handy article that covers how to make your SVGs accessible with alt text if you'd like to learn more.Other than that, this overall looks pretty good! Your version seems a little stretched out horizontally compared to the design, so you may want to play with how it looks at a variety of screen sizes as well π
@Chockplay
Submitted
Thanks for view my post. Please fell free to provide feedback, recommentadions, suggestions or code optimizations to improve this project.
Have fun building!!!
@Theosaurus-Rex
Posted
Hey @Chockplay, congrats on completing the challenge!
This looks pretty good - a couple of small suggestions to help improve accessibility and semantics
<h1>
, but in this case the component is isolated, so having a <h2>
with no <h1>
doesn't make much sense semanticallyOtherwise, great work! β¨
Marked as helpful
@Madeline0421
Submitted
@Theosaurus-Rex
Posted
Hey @Madeline0421, great work! Looks very close to the design!
A couple of suggestions for your HTML that will make it more accessible:
alt=""
is perfectly acceptable here.<time>
tag - this tag can be provided a datetime
attribute, which takes in the publishing date in a machine-readable format. In a real-world scenario, this will help search engines to rank your website content! You can read more about this tag on W3Schools if you'd like to learn more.<p>
tag is preferable, and denotes hierarchy within the markup contentOtherwise, awesome work! I hope this helps!
Marked as helpful
@destastan
Submitted
@Theosaurus-Rex
Posted
Hey @destinyou, looks really great!
One small thing that would improve accessibility is changing your <h2>
to a <h1>
, as it's the top of the informational hierarchy on the page.
You could also potentially use a <time>
tag to encapsulate the publishing date - this isn't always needed, but in a real world scenario where this card is part of a larger blog, it helps search engines to index your page better when returning search results. You can find more information on the specifics over at W3CSchools if you'd like to try it!
Other than that, great work! I love how smooth the hover animation is π
@FatehaRsd
Submitted
@Theosaurus-Rex
Posted
Hey @FatehaRsd, great work on your solution! A couple of small suggestions:
Other than that, fantastic work! Hope this helps π
Marked as helpful
@NeuralG
Submitted
-Thats my first responsive website so any feedback would be appreciated
-What is the best way to decide between buttons and anchor links? I used anchor links here but I'm not sure about that decision.
@Theosaurus-Rex
Posted
Looks great! Very close to the original design!
Regarding your question about the difference between anchor tags and buttons, you've made the right choice here by using anchor tags. The key difference is that an anchor tag is used to navigate to a web link, whereas buttons are typically used when a user is making some kind of decision when interacting with a web interface, e.g. a "Submit Form" button, clicking a close button on a popup alert, etc.
Hope this helps!
Marked as helpful