Rachel Cessna
@cessnar516All comments
- @cessnar516Submitted about 3 years ago@cessnar516Posted about 3 years ago
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.
0 - @HaithamAmirehSubmitted about 3 years ago
If anyone can help me on how to make the stats section better. Thank you.
@cessnar516Posted about 3 years agoI 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>
- One more thing that would help your accessibility would be to use some semantic HTML elements to define the sections of your page. I suggest adding a
<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 helpful1 -
- @vasilisalmpanisSubmitted about 3 years ago
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
@cessnar516Posted about 3 years agoCongratulations on completing your first project! Here are a few things that will help with your accessibility:
- Make sure you use heading's sequentially. Right now "Order Summary" is an
<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. - Try to incorporate semantic HTML elements to define the structure of the page. For example, you need a
<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. - If an image is decorative (I would say all three of the images in this layout are), set the alternative text to empty quotes:
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:
- instead of creating a
<div>
element to contain the background image, you can add the background image directly to the<html>
or<body>
element by using something likebody { 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 abackground-color
property with the pale blue color to get the layered background shown in the design images. - I would also explore using percentage widths for your links at the bottom of the card and using margin/padding to position it instead of using relative positioning. The relative positioning causes your layout to break when the browser window is resized.
- Try adding a link to
<head>
in yourindex.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 helpful1 - Make sure you use heading's sequentially. Right now "Order Summary" is an
- @rajbindersandhuSubmitted about 3 years ago
Please advice if I could improve in CSS and HTML , to make my code more efficient and clean ?
@cessnar516Posted about 3 years agoThe 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:
- It looks like like you're missing the Google font called for in the style guide, so you'll need to add a link to that stylesheet in your HTML head and set the
font-family
for thebody
. - Add an empty
alt
attribute to all of the images since they are decorative:alt=""
. - Try using semantic HTML elements to define the sections of the page. I suggest at least adding a
<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!
0 - It looks like like you're missing the Google font called for in the style guide, so you'll need to add a link to that stylesheet in your HTML head and set the
- @luiseduardoaugustoSubmitted about 3 years ago
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.
@cessnar516Posted about 3 years agoHi 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
tospace-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!
0 - @vtrev05Submitted about 3 years ago
Every comments are welcome!
@cessnar516Posted about 3 years agoHi 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 helpful0 - @R0b3rtGSubmitted about 3 years ago
Please leave some feedback! :)
I would really appreciate it!
:)
@cessnar516Posted about 3 years agoGreat 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 usingborder-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 helpful0