@cessnar516
Submitted
@cessnar516
@cessnar516
Submitted
@cessnar516
Posted
The report generated for my solution is flagging my use of aria-label on the social icon links; however, they are used properly. The aria-label is used to provide accessible text for screen readers since the links only contain icons.
@HaithamAmireh
Submitted
If anyone can help me on how to make the stats section better. Thank you.
@cessnar516
Posted
I think your solution looks really nice! Here are few suggestions to make your layout more accessible:
Heading levels should be used sequentially on a page. Right now, you have the card title set to an <h1>
, so the next heading on the page should be an <h2>
instead of an <h3>
, and the next heading after that should be an <h3>
instead of an <h6>
. It's best to use the headings sequentially, and then adjust the CSS of the heading to be the size/style you want because screen reader users depend on the structure of the page to understand it's content. When heading levels are skipped, it can be confusing, and those users may think they're missing information.
To make the HTML for the stats section more semantic, I suggest using a <ul>
instead of using headings because the information is a list of statistics instead of sections of content. Then, you can style the <ul>
to remove the bullets and display horizontally. Your HTML might look something like this:
<ul class="stats">
<li><span>10k+</span> Companies</li>
<li><span>314</span> Templates</li>
<li><span>12M+</span> Queries</li>
</ul>
<main>
element around your main-card div, and a <footer>
element around the attribution information at the bottom of the page. These elements will provide helpful information to assistive technology to help those users understand the structure of the page.Hope this helps!
Marked as helpful
@vasilisalmpanis
Submitted
This is my first project with css and html, I am a complete beginner. Please leave some comments and let me know what I could have done better
@cessnar516
Posted
Congratulations on completing your first project! Here are a few things that will help with your accessibility:
<h1>
, so "Annual Plan" should be an <h2>
instead of an <h3>
, and the price should be an <h3>
instead of an <h4>
if you want it to be a heading. Personally, I would suggest changing the price to a <p>
since it doesn't define a section of information on the page.<main>
to define the main region of the page - I suggest adding it directly after the <body>
element and having it contain everything except the footer. Then, place the footer information in a <footer>
element to define that region as well. The semantic HTML element help explain the markup of the page to individuals who use assistive technology, so they're very important.alt=""
. This will let a screen reader know that they can skip over the image because it doesn't provide any important information. Otherwise, the screen reader will read "no picture" or "illustration" when it gets to the images, and that can be confusing.And here are a few suggestions for your layout:
<div>
element to contain the background image, you can add the background image directly to the <html>
or <body>
element by using something like body { background-image: url("css/background.svg");}
. Just make sure the url text is the path that points to your image. And then you can also add a background-color
property with the pale blue color to get the layered background shown in the design images.<head>
in your index.html
file for the Google font stylesheet specified in the style guide to get your fonts to match the example.Hope this helps!
Marked as helpful
@rajbindersandhu
Submitted
Please advice if I could improve in CSS and HTML , to make my code more efficient and clean ?
@cessnar516
Posted
The overall layout looks like it's mostly there - just needs a few tweaks to get it to match the example and take care of the HTML and accessibility issues highlighted in the report for your solution. Here are a few suggestions:
font-family
for the body
.alt
attribute to all of the images since they are decorative: alt=""
.<main>
inside of the body element and then adding a <footer>
element to contain the .attribution
information. You could change your .order-card
div to a <section>
element too and replace your current sections inside of the .change-box
with divs instead because section elements require a heading.Hope this helps!
@luiseduardoaugusto
Submitted
I had trouble with tthe value proposition. Aligning to left the icon, title and price, and at the right the link.
It will be amazing to have feedback.
@cessnar516
Posted
Hi Luis!
To get the value proposition to align properly, you will need to make a few adjustments to your HTML markup. The icon, title, and price need to be grouped together so you can change justify-content
to space-between
. I suggest formatting your HTML like this:
<div id="proposal">
<div id="proposalTitle">
<img src="/images/icon-music.svg" alt="">
<h2>Annual Plan</h2>
<p>$59.99/year</p>
</div>
<a href="#">Change</a>
</div>
This way, the #proposalTitle div and the link are both children of the #proposal div. Then, you can set #proposal { justify-content: space-between; }
to get the items to align on opposite sides of the card. You will also need to adjust padding and margins to adjust the spacing and keep the information from being up against the edge of the card.
One thing that will help clean up your code some too would be to avoid using divs and IDs unless you have to have them. So rather than wrapping each element in a div and adding an ID, you can add the class or ID to the element itself, or you can add the styling directly to the element without using an ID or class. For example, instead of adding a class to the h2 inside of the #proposal div, you could style the h2 by using the selector #proposal h2
.
Hope this helps!
@vtrev05
Submitted
Every comments are welcome!
@cessnar516
Posted
Hi VetrV,
One thing that may help simplify your code would be to target the elements inside of the cards instead of adding classes to them. For example, right now you have <h3 class="cardTitle">
and you're targeting .cardTitle
in your CSS. Instead, you could remove the class and target .singleCard h3
. This would eliminate several classes from your HTML.
You may also want to try using CSS Grid to align the cards instead of Flexbox. Grid will allow you to get the intended layout without having to relatively position each of the cards, which would cut down on your CSS a lot. Here's a great resource for Grid if you want more information: CSS Tricks - A Complete Guide to Grid
One more note related to accessibility - right now, you're jumping from the page title <h1>
to <h3>
in the cards which can be confusing for individuals who use screen readers and those with cognitive disabilities. Screen reader users have the option to hear a list of all the headings on a page, and the technology will tell them what level each heading is. If they're listening to the list, and a heading level is skipped, they may think they have missed some information. For this reason, I recommend changing your card titles to <h2>
so your headings are sequential.
Hope this helps!
Marked as helpful
@R0b3rtG
Submitted
Please leave some feedback! :)
I would really appreciate it!
:)
@cessnar516
Posted
Great job, Robert! Your page is responsive, and I like that you used CSS grid to create the desktop layout. One thing you could do differently would be to use semantic HTML elements for as much of your layout as possible instead of using divs. For example, right now all of your cards are divs. Instead, you could make each card an <article>
element. The top color can be added to the card using border-top
, so you wouldn't need the <div class = "top-color">
anymore, and then you could add your padding to the card itself instead of needing the <div class = "content">
. This would leave you with a <article>
element containing an <h3>
, <p>
, and <img>
- no more divs!
Hope this helps!
Marked as helpful