Skip to content
OverviewSolutions
18
Comments
14
Shawn Lee
@OGShawnLee

All comments

  • Gareth•410
    @Gareth-MooreSubmitted over 2 years ago

    Hello there,

    I restarted the project 2 times as I didn't really get a solid grasp of what the html layout should be. I finally settle with what I have now and it's working well.

    I had particular difficulty getting the list items to align properly.

    Any feedback is highly appreciated. In fact I'll use voodoo to protect you from ghosts if you help me! Great deal I'd say!

    P.S If you DM me I'll send you my discord, I'm putting together a little community of active front end developers who are looking to improve their skills. So if you'd like to join feel free.

    The ULTIMATE PATIENCE tester 5000! Using JS to hurt myself today...

    2
    Shawn Lee•560
    @OGShawnLeePosted over 2 years ago

    Hey. Shawn here. I am glad you managed to make it work! My solution took a lot of time and no time at the same time.

    Here is some feedback for you:

    An Accordion is a widget that should follow a pattern documented in the ARIA Authoring Practices Guide. Which would be something like this:

    An Accordion has a structure similar to this.

    <Accordion> -> div
  <AccordionItem> -> div
    <AccordionHeader> -> (h1-h6) or element with aria-role="heading" and aria-level={1-6}
      <AccordionButton> -> button
          This toggles the Panel
      </AccordionButton>
    </AccordionHeader>
    <AccordionPanel> -> div
        This is the content of your Accordion Item.
    </AccordionPanel>
 </AccordionItem>
</Accordion>

    The Button must have aria-controls set to the id of the Panel it controls, when it is visible. It also should have aria-expanded set to either true or false. Should be the only child of the Header.

    The Panel should have aria-labelledby set to the id of the Button, and might have aria-role set to region.

    And optionally, the user should be able to move focus around the Buttons using the Arrow Keys from the keyboard (ArrowUp - ArrowDown).

    This challange was easy for me because I had already built an Accordion component for my headless component library for Svelte. All I had to do was install, import and style. If you havent tried Svelte you should try as it is really beginner friendly and easy to use, and maybe you could check out my library xd. I am working on version 0.9.0 which will have a massive refactor and a new component.

    I hope this is useful for you and best wishes. Have a great day mate!

    ps: I think you should upload your projects in an individual repo so that it is easier to check them and quickly know what languages or frameworks you used.

    ps2: here is the link of the Accordion pattern! https://www.w3.org/WAI/ARIA/apg/patterns/accordion/

    0
  • Shawn Lee•560
    @OGShawnLeePosted over 2 years ago

    ¿Ey que pasa? Según tu perfil, eres de Perú así que te voy a escribir en Español colega xD.

    Bastante buena la solución! Se ve genial en todos los tamaños de pantalla. Solo que yo le pondría un poco mas de padding a los paneles con las estrellas en desktop, se ven un poco flaquitos en mi opinión.

    Sobre los testimonios, yo usaría un article como tag contenedor y usaría el nombre de las personas como el heading. En este caso sería un h3, pero para ello pondría un h2 para toda la section de los testimonios y lo haría sr-only para que solo lo vean screenreaders. Algo así:

    <section>
  <h2 class="sr-only">Testimonials</h2>
  <div>
      <article>
        <h3>/* nombre */</h3>
      </article>
 </div>
<section/>

    Así usas HTML semántico y es mucho más fácil de leer y entender el código.

    Eso es todo mi feedback. Buena solución colega. Ten un excelente día! :D

    Marked as helpful
    0
  • Shawn Lee•560
    @OGShawnLeePosted over 2 years ago

    Hey mate. How is it going?

    I checked your project and I have some feedback for you.

    • I suggest wrapping all of your content inside of a main tag.
    • Every page should have an h1. And I would have used a span instead of the h5. By the way headings should go down in order. If you have and h1 then the next one in your page should be and h2 then h3 and so on and so forth. Doesn't apply if your are going up, though.
    • Always have and alt attribute in your images, you can leave them empty if the image is a decoration.
    • I would use this to align your card properly and correct the background pattern and color.
    body {
  background-image: url("../images/pattern-background-desktop.svg");
  background-repeat: no-repeat;
  background-color:  hsl(225, 100%, 94%);
  background-size: 100%; /* the pattern stretches the whole view width*/
  display: flex;
  flex-direction: column;
  align-items: center;
  justify-content: center;
  gap: 3.5rem; /* separation between the card and attribution */
  min-height: 100vh;
}

    I hope this helps, mate. Have a great day!

    Marked as helpful
    1
  • Shawn Lee•560
    @OGShawnLeePosted over 2 years ago

    Hello! How's it going? You did an excellent job!

    Here is some quick feedback to you. Since I am not a big fan of plain CSS or SCSS I won't help you there sorry :'( Instead I will help you out with the Accordion implementation.

    • Accordion Button should be nested inside of a heading or an element with role set to heading and an appropiate aria-level, and it should be the only child element. Something like this:
    <h2>
   <button> Your Accordion Item Title Here! </button>
</h2>
<div role="heading" aria-level="2">
   <button> Your Accordion Item Title Here! </button>
</div>

    • Accordion Button should always have aria-expanded set to true (panel is visible) or false (panel is not visible).

    • Each Accordion Button and its related Accordion Panel should be linked to each other: the button should have aria-controls set to the id of the Accordion Panel (when the Panel is visible) and the Panel might have aria-labelledby pointing to the button id. Something like this:

    <h2>
  <button id="button-1" aria-controls="panel-1"> Accordion Button </button>
</h2>
 <div id="panel 1" aria-labelledby="button-1">
  Accordion Panel
 </div>
    • An Accordion may implement navigation using the vertical keyboard arrows (ArrowUp - ArrowDown). This is optional.

    I recommend looking at the WAI-ARIA Authoring Practices. It explains all the behaviour and accessibility you need to build cool HTML patterns like accordions, dialogs, menus, tabs, etc.

    Hope this helps you. Keep working on your projects and have fun. Best wishes!

    Marked as helpful
    0
  • Rocky Gurung•50
    @grgrockySubmitted over 2 years ago

    I used flex layout and used :nth-child() selector to give style to each <p> element. I wonder if it is a long process??

    qr-code using html and css

    1
    Shawn Lee•560
    @OGShawnLeePosted over 2 years ago

    Hey. Good job! Here's my feedback for you mate.

    • Yep. You can target both paragraph using a common class and add styles that they share. However you should have not added two paragraphs, instead you could have used an h1 for the big boi and a paragraph for the small one.
    • Your markup should have an h1 which would be the one I mentioned.
    • You images must have alt attributes, you can leave them empty if the image is used for decoration.
    • I think you should have used CSS variables to store the theme from the the style guide.
    • For centering your content I would have coded something like this.
    body {
  display: flex;
  flex-direction: column;
  align-items: center;
  justify-content: center;
  gap: 7.5rem;
  font-family: 'Outfit', sans-serif; /* apply the font globally here */
  min-height: 100vh;
}

    And removed the margin from the #container selector.

    Hope this helps mate. Have a great day!

    Marked as helpful
    1
  • Shawn Lee•560
    @OGShawnLeePosted over 2 years ago

    Hey! Here's some feedback for you.

    About HTML:

    1. All of your main content should be wrapped inside of a main tag.
    2. Your page should have an h1 tag. Change the h3 for an h1 and its solved.
    3. Image tags must have an alt attribute. Which can be left blank if the image is used for decoration.

    About CSS:

    • Add someting like this to your body. This will center your content properly regardless of viewport.
    body {
  display: flex;
  flex-direction: column;
  align-items: center;
  justify-content: center;
  gap: 7.5rem; /* this will add some nice gap between your card and the attribution */
  min-height: 100vh;
}
    • Remove this from you card: transform: translateX(515px);
    • Use relative units like rem instead of px.

    I am not a plain CSS fan so I can't provide much feedback there, yikes: T-T

    Hope this helps. Have a great day!

    1
  • Doston Nabotov•950
    @dostonnabotovSubmitted over 2 years ago

    Hi, everybody! I decided to update this challenge for final touches and bug fixes. I would be grateful if you could give some feedback or show me where I made mistakes.

    Social Media Dashboard

    1
    Shawn Lee•560
    @OGShawnLeePosted over 2 years ago

    Hey Shawn here. Your solution looks great. Good job!

    Here is some feedback:

    • For the border background you can use a pseudo-element and do something like this:
      /you-card-selector/ {
    position: relative;
  }

  /your-card-selector/::before {
    content: '';
    position: absolute;
    top: 0;
    left: 0;
    right: 0;
    height: 0.25rem;
    background-color: /your colour here/;
    border-top-radius: 0.125ex;
  }
    • For the theme toggler I would have used a button instead of an input.
    • Your headings levels should be in order. In your HTML, make sure the next heading is one level below the previous one: LIke h1 -> h2 -> h3. Your markup has an h1 at the top and then an h3 in your first four cards . Placing an h2 in the top card section container should fix the accessibility issue from the report.

    Hope this helps mate. Have a great one!

    Marked as helpful
    1
  • Arthur•290
    @ArthurPogSubmitted over 2 years ago

    Due to the fact that this is my first project, my biggest problem with it was aligning everything vertically. I came up with a solution but I have no idea if it is an elegant one. I have a lot of work ahead of me.

    QR Code Component Solution

    1
    Shawn Lee•560
    @OGShawnLeePosted over 2 years ago

    Hello, good one! Some feedback right here.

    1. You should have wrapped your content inside of a main tag.
    2. About the alignment you could have aligned it with flexbox like this:
    body {
  min-height: 100vh;
  display: flex;
  align-items: center;
  justify-content: center;
}

    1. For transparent colors I recommend using rgba because it is easy to know how transparent a color is by looking at the last number. Like rgba(0, 0, 0, 0.25).

    2. Use CSS variables. They come in handy when you need to change something. Update once and everything changes nicely.

    This is a pretty simple project and you did a great job. Hope this helps and you keep coming back with new solutions! Have a good one :)

    Marked as helpful
    1
  • Shawn Lee•560
    @OGShawnLeePosted over 2 years ago

    Hey. You did well!

    1. Your text is a little larger and/or the witdh of your card is narrower.

    2. No. I don't think so. As far as my eyes can tell.

    3. It looks mostly fine. You should wrap your content inside of a main tag and give the image an alt attribute where you give a description of the image.

    About the styling:

    1. You should used the body to center the card instead of giving it margin-top. You can correct it by displaying the body flex, add align-items: center; justify-content: center; and give it a min-height of 100vh.

    2. Use rem units instead of pixels. Maybe the box shadow is ok with pixels, I use pixels there as well :D

    3. I don't think a nested container was needed nor giving it and the text a fixed width. I would have used a flex column container for the whole card and an inner container with padding for the heading and text.

    Hope this helps. Have a good one!

    Marked as helpful
    1
  • SuKyung Kim•60
    @sukyungdevSubmitted over 2 years ago

    Hi everyone! I have some questions.

    Question 01.

    It was difficult for me to make a background for this challenge. Is there no problem with the background I made?

    Question 02.

    Is it okay to have another section tag inside the section tag?

    Your comment are very helpful to me. I will be waiting your comment.

    Any other feedback is welcome. Thank you!

    Order Summary component using CSS

    3
    Shawn Lee•560
    @OGShawnLeePosted over 2 years ago

    Hello!

    Your solution looks almost like the design!

    1. You had to use both versions of the background, one for desktop and another one for mobile. You could have done it using a picture tag and source tags inside of it. Add to each source tag the src of the image and a media attribute that informs the browser of when that image should be rendered based on the size of the page.
    2. Yep! Sections are allowed to be nested inside of other sections. However you have to keep in mind is that they must have a heading (h2-h6) inside of it in order to inform screenreaders what that section is all about.

    Other than that your solution is all clear! Hope this is useful.

    Marked as helpful
    1
  • Ryani Durham•20
    @ryanimdSubmitted over 2 years ago

    Does anyone have any tips for centering my button without relying on margin? I used margin-left because that's just what ended up working for me, but I'm curious as to what best practice would be for that. Any and all constructive feedback welcome!

    Advice Generator

    2
    Shawn Lee•560
    @OGShawnLeePosted over 2 years ago

    Greetings. Brilliant work you did there!

    For the button I would use absolute positioning:

    1. position: relative to the container
    2. position: absolute to the button
    3. bottom: (around -1.75rem idk); + left: 50%; + transform: translateX(-50%)

    That should get your button more or less where it should be.

    Keep learning and improving. Hope this helps! :)

    0
  • Jamiu Muritala•100
    @murytarlahSubmitted over 2 years ago

    I had a bit of difficulty with the filtering section but thanks to stack-overflow, I was able to find a solution Question: I re-render the countries grid view every time a key is pressed in the search input button looping through 250 each time. I feel this is bad practice but can't think of a better way to go through this, any help would be appreciated

    REST Countries API with color theme switcher using REACT and Tailwind

    2
    Shawn Lee•560
    @OGShawnLeePosted over 2 years ago

    Hey Jamiu. Good job!

    If you have to build a fairly simple React project I highly recommend using the Vite React template instead. It is much faster than Create React App because it only downloads the core dependencies and on top of that Vite is really fast for hot module replacement and offers other cool features.

    About the input maybe you should use debouncing or an onChange event instead of keydown.

    Good luck!

    Marked as helpful
    0