Skip to content
OverviewSolutions
34
Comments
24

Ahmed El_Bald

@Ahmed-ElbaldEgypt - New Cairo1,020 points

They see the web as elements stacked over each other. I see it as a book of titles and chapters, a drawing of outlines and colors, a machine of inputs and outputs, and a chance to show how things could be built.

I’m currently learning...

I'm currently learning Angular to use it as my default frontend framework. I'm also interested in HTML accessibility and code structure.

Latest solutions

View more solutions

Latest comments

  • Ahmed El_Bald•1,020
    @Ahmed-ElbaldPosted about 1 year ago

    Alright,

    I just want you to know one thing: Before you try to implement the design, consider the markup. Before you write your HTML, think about what each element represents.

    In my opinion, the whole design consists of two things: Information about my storage and some buttons to import and export this data. Thus you should consider the following when writing your markup:

    • In your .controls-box, you can't use divs where you must use buttons. Why? because they are controls, they have functionality:

       // THIS IS WRONG
 <div class="document">
   <img src="images/icon-document.svg" alt="" aria-hidden="true">
 </div>

 // DO THIS
 <button type="button">
   <img src="images/icon-document.svg" alt="Save your data">
 </button>

    • It would be great if you could present storage information using a description list (dl). I know It would be hard to match that with the design. But I would do that under any cost using the .visually-hidden class and the aria-hidden attribute.

    Marked as helpful
    2
  • Ahmed El_Bald•1,020
    @Ahmed-ElbaldPosted about 1 year ago

    Hello there,

    I think you did a great job with this challenge. I just have some notes that might find helpful:

    • You need to increase spacing between elements a little bit as, right now, your elements are crushed into small space.

    • Your solution lacks some features when it comes to accessibility. You should wrap the whole card in an article element as it seems like a standalone element (it gives meaning on its own).

    • In a real-world application, the text "Equilibrium #3429" won't be your h1. Therefor, I suggest using an h2 or h3 for that.

    • When presenting the price and the days left, you can use a dl element whereas hiding the terms using a .visually-hidden class, so that it makes sense for disabled users. It should be something like that:

      <dl>
  <dt class='visually-hidden'>Product price</dt>
  <dd>0.041ETH</dd>
  <dt class='visually-hidden'>days left</dt>
  <dd>3 days left</dd>
</dl>

      You can find more about the dl element here. You can also read about the .visually-hidden class here

    Marked as helpful
    1
  • Ahmed El_Bald•1,020
    @Ahmed-ElbaldPosted about 1 year ago

    Al Salam 3likum Abdulrahman,

    • Regarding your question about the empty space at the bottom, I think you just forgot to set the line-height property on your fifth card's quote. The problem is in this CSS rule:

       // Try adding (.card-5 .review-quote)
  .card-1 .review-quote, .card-2 .review-quote, .card-4 .review-quote {
  color: rgba(255, 255, 255, 0.75);
  line-height: 1.65;
 }
 // More better, you can do something like this:
  .card .review-quote {
  // Your props here
}

    I've noticed some other issues concerning accessibility and code structure. If you are interested in solving them, just let me know.

    Thanks.

    Marked as helpful
    1
  • Ahmed El_Bald•1,020
    @Ahmed-ElbaldPosted about 1 year ago

    Hi Alain,

    Regarding your question about better CSS, I wanna say there's no best way for doing that, but you should follow these guides in general:

    • I see you are using Bootstrap. But if you are at the beginning and you want to write cleaner CSS, you should try to make things on your own. Practice makes perfect. When you reach a point where you think you've got a good grasp of CSS, then you can use libraries like Bootstrap and Tailwend.

    • Try making use of CSS Custom Properties (Variables) as they make your code reusable

    • When adding class attributes, try to make them as descriptive as possible. For instance, class attribute with values like "div", "one, two" or any generic name are not preferable. You can also use the BEM Naming Convention

    • Kevin Powell has a great channel that is mainly interested in CSS stuff. Personally, this channel helped me a lot.

    Besides that, there are some accessibility issues that I can mention if you are interested.

    I hope you find that helpful.

    Marked as helpful
    1
  • Ahmed El_Bald•1,020
    @Ahmed-ElbaldPosted about 1 year ago

    Hi Asma,

    Again, I do believe you are really talented when it comes to applying the UI. I like how it's responsive on all screen sizes. I just have some notes here that you might find useful:

    • Text like '30-day, hassle-free money back guarantee' should not be wrapped in an h2 element as it's not actually a page heading. Use a regular p element instead as headings imply the beginning of a new section.
    • Try to make your classes names more descriptive so that other developers can test your code more easily. For instance, you can replace your one, two and three classes with intro, cta (call to action) and outro or anything that is meaningful.
    • Again, you should've used a description list dl for the pricing.
    Marked as helpful
    0
  • Ahmed El_Bald•1,020
    @Ahmed-ElbaldPosted about 1 year ago

    Hello Osman,

    So I think your solution is great regarding the UI and even some of the accessibility features. I've noticed you are using the article element for your card, which is great. However, there are some subtle notes that you might find helpful:

    • The alt text of your image should be descriptive. Placing the text (Perfume Image) as the alt text is quite generic. Instead, try using a longer version that clearly describes the image. Something like: 'Gabrielle Essence Eau De Parfum surrounded by tree leaves....etc'. You can read more about the alt attribute here

    • In the price section you can replace your div.felx-group with a dl which can take key-value pairs to associate them together. That would be something like this:

    <dl>
  <dt>Product Price</dt>
  <dd>$149.99</dd>
  <dt>Original Price</dt>
  <dd>$169.99</dd>
<dl>

    Thank you.

    Marked as helpful
    0
View more comments