Skip to content
OverviewSolutions
10
Comments
10
John Idenyi
@JohnIdenyi

All comments

  • John Idenyi•250
    @JohnIdenyiPosted over 2 years ago

    Hi you did pretty good for first challenge.

    • You used inline and internal CSS to style your webpage. It is okay to use internal css for a single page but is not advisable you use it for a website with multiple pages. You shouldn't use inline CSS in styling your document. The recommended way which is the best practice is to use external stylesheets.

    • Your texts don't have space around them making them touch the container. Add a padding property to the div wrapping the texts.

    • You have some accessibility issues. You don't have an h1 element in your document. Change the first paragraph to an h1 element. Every page must have an h1 element because screen readers make use of it to identify the main content to visually-impaired persons. It also helps in better optimization by search engines.

    • You should use semantic HTML elements like <main> or <section> to wrap contents in your document. So instead of this <div class="center"> you should do this <main class="center"></main>.

    I hope this was helpful!

    Marked as helpful
    1
  • John Idenyi•250
    @JohnIdenyiPosted over 2 years ago

    Nice work! Though you have some accessibility issues.

    • You don't have an h1 element in your document. Change the h2 to an h1 element. Every page must have an h1 element because screen readers make use of it to identify the main content to visually-impaired persons. It also helps in better optimization by search engines.

    • Your html root element is meant to have a lang attribute set to a value, for example lang="en" indicates the page is in english.

    I hope this was helpful.

    Marked as helpful
    1
  • Adam Wozniak•230
    @adamwozhereSubmitted over 2 years ago

    I'm unsure as to the correct way of implementing accessibility for this project; whether a figure with figcaption may be better as a way of describing the purpose of the image to screen-readers -- advice would be appreciated!

    QR code component - CUBE CSS, custom properties

    1
    John Idenyi•250
    @JohnIdenyiPosted over 2 years ago

    Hi, you did good for this challenge. You don't have an h1 element in your document that is why there is accessibility issue. Change the h2 to an h1 element. Every page must have an h1 element because screen readers make use of it to identify the main content to visually-impaired persons. It also helps in better optimization by search engines.

    Marked as helpful
    1
  • John Idenyi•250
    @JohnIdenyiPosted over 2 years ago

    Nice work Pawel! But you have some accessibility issues.

    • <p class="creation">Creation of <x>Jules Wyvern</x></p> "x" is not an html element so it shouldn't be used rather use <span> which is meant for inline elements.

    • Secondly, you should use semantic HTML elements like <main> or <section> to wrap contents in your document. So instead of this <div class="background"> you should do this <section class="background"></section>.

    • Thirdly, your webpage is not responsive on mobile device. Try using a media query to create styles that apply to mobile devices. You should try reducing the width of the container and the background class.

    I hope this was helpful.

    Marked as helpful
    0
  • John Idenyi•250
    @JohnIdenyiPosted almost 3 years ago

    Hey, great design! I think the only thing to point out is it is not responsive. Add a media query so it can scale properly on a mobile view

    Marked as helpful
    0
  • John Idenyi•250
    @JohnIdenyiPosted almost 3 years ago

    Hi, you did pretty well for first challenge. I would suggest using flexbox to center everything on the page and also try reducing the width of the the .background-middle class.

    0
  • Luca Toni•80
    @shavedheadSubmitted almost 3 years ago

    Let me know what you think! I had a lot of trouble with the background images. Let me know what I could have done better. Thank you for your responses!

    Profile Card Component

    2
    John Idenyi•250
    @JohnIdenyiPosted almost 3 years ago

    Dope work!

    0
  • GrischK•140
    @GrischKSubmitted almost 3 years ago

    Hi guys. I updated my solution and the site preview is conform but the design comparison isn't. It still shows my previous solution. Is there a way to fix it ? Thanks.

    QR code component

    1
    John Idenyi•250
    @JohnIdenyiPosted almost 3 years ago

    Hey, You did pretty good. Create a margin between your paragraphs and image to avoid clustering. I think you did well man.

    0
  • John Idenyi•250
    @JohnIdenyiPosted almost 3 years ago

    Hi, nice work. I would suggest increasing the width of the .qrandtext class element to 25% or more and also set a media query and increase the width to 85% for the same class so it can scale well on mobile devices.

    Marked as helpful
    1
  • Iva•120
    @IvuskaSubmitted almost 3 years ago

    Hi everyone, greetings from Prague,

    I hopefully fixed all the stuff so it works on mobiles properly now.

    Would be really great if someone can review the code itself.

    Thank you a lot, Iva

    Mobile first solution using flexbox for QR component

    3
    John Idenyi•250
    @JohnIdenyiPosted almost 3 years ago

    Nice work!

    0