Skip to content
OverviewSolutions
23
Comments
22
Walter Sono
@waltersono

All comments

  • DIICA99•115
    @DIICA99Submitted over 3 years ago

    Hello everyone!

    I've just completed my second challenge and I'll like to have some feedback. Please, it really helps me to improve my skills and knowledge.

    1 - Does the site looks good? 2 - After taking a look at the source code, what do I have to improve? 3 - Average from 0 to 10. 4 - Do I have any chance to be a wonderful frontend dev?

    Intro component with full validation signup form (HTML, SCSS, BEM, JS)

    2
    Walter Sono•660
    @waltersonoPosted over 3 years ago

    Hi there DIICA99

    Nice work! The design looks good on almost all screens. You definitely have a chance to be an awesome developer

    I am going to give many tips you need for this and your next projects, here we go:

    HTML

    Use Semantic HTML

    • Using the correct tags to structure your page helps the Web, browsers, search engines, users to understand, categorize, list your website
    • Put the heading section inside a <header> tag

    Code for web accessibility

    • Help screenreaders navigate your page easily
    • Remove the empty labels and insert an aria-label attribute inside the <input>
    • Provide an alt text for each image

    CSS

    Use low specificity

    • Try as much as possible to keep to target your elements with class (.heading) instead of targeting them by structure or tag name (input,section form input)
      • If you target by structure, then you will have to change the CSS every time you change your HTML code
      • If you target by tags, then you will run into problems when you want to apply different styles to the same HTML input

    JavaScript

    • Use strict mode to enforce some good practices
    • Call the javascript file on the top inside the <head> with defer attribute, instead of calling it at the bottom for performace

    There you go my fellow mentor.

    Hope this is helpful to you.

    See you next time, feel free to go and checkout my work as well.

    Marked as helpful
    1
  • Walter Sono•660
    @waltersonoPosted over 3 years ago

    Nice job domingoslatorre completing this challenge

    It looks very good, the mobile and desktop.

    You used the corrects HTML tags and even tried to support as much attributes for accessibility, very cool.

    Just as a suggestions:

    • Reduce the min-width

      • Because the mobile version start when there is still space for the design to be in desktop

    • Correct few things in BEM

      • page__social__item is good practice in BEM
      • A element can not have another element:
      • "An element is always part of a block, not another element. This means that element names can't define a hierarchy such as block__elem1__elem2." Quick start

    That is it, my fellow mentor.

    Maybe you could help each other with suggestions.

    Marked as helpful
    1
  • Walter Sono•660
    @waltersonoPosted over 3 years ago

    Hi Jugo-JS

    The design looks good, just some suggestions to make your code better in the next projects:

    • Do not use images tags for background images

      • Images tags are used to insert content relatable images
      • Images inserted using images tags interrupt other resources
      • Use divs instead, and apply a background images

    • Add alternative text the image of the profile photo

      • Alternative text are used by screen readers to inform a user that cannot see the image what is the image is about, or even when the user as slow internet and the image cannot be downloaded

    Ok Jugo-JS, hope this is helpful.

    Keep on coding!

    Marked as helpful
    0
  • Walter Sono•660
    @waltersonoPosted over 3 years ago

    Hi AnkitLamsal

    Nice work completing your first challenge.

    Here are some points to improve your next projects:

    • Add a transitions on when the user hover the button

      • This will make a smooth experience on button hover
      • Just add this to your button transition: all 0.2s

    • Target your elements by class not by element

      • This will make your projects easy to maintain because
      • If you had given the a tag a class you could have style it with fewer lines, like so:

    HTML <a href="#" class="button">Learn more</a>

    CSS .button { *styles go here* }

    • Learn BEM Naming convetion
      • Has a future professional, you want to write clean code, easy to understand and easy to maintain, writing your code following a naming convention is a good practice

    That is it my fellow mentor

    Hope this is helpful, and keep on coding!

    See you!

    0
  • David-ODB57•10
    @David-ODB57Submitted over 3 years ago

    Feel free to tell me what's wrong or would better to do with my solution if you see something.

    Flexbox solution

    2
    Walter Sono•660
    @waltersonoPosted over 3 years ago

    Hello David-ODB57

    Nice work, the design looks good and you have organized your code. That is good.

    Here are my suggestions:

    • Add a border to the buttons before hover

      • Because you added a border on hover, when you hover the button it grows and does not look good

    • Add a transition on button hover

      • This will make the hover effect smooth

    • Fix the attribution text

      • Instead of making the body flex, add a container and make it flex
      • Then move the attribution text outside of this container to it can be below of your solution

    • Add media queries at the right breakpoints

      • I believe the best breakpoints to add mediaqueries are when your design starts to break
      • You design starts to breaks at 1116px but you only added at 600px, this is not good
      • Dont rely on devices dimensions because that changes every time, you do not want to go back to your websites to change your break points every time a device with new dimensions comes along

    • Provide an alternative text to each image

      • Alternative texts help screen reader users to understand your content despite their difficulty to see

    • Use links for links and buttons for buttons

      • I think that instead of using a button you should have used a link
      • Imagine if this was a real website, you would have write a JS script or backend script to be able to redirect the user to a "learn more" page

    Ok, this is a lot

    But that will make your next projects better.

    Hope this is helpful.Keep the good work

    Marked as helpful
    1
  • sujeet kumar chaudhary•30
    @susu548Submitted over 3 years ago

    I am learning css so i have done static part of the design . so to make it responsive should i use media queries or bootstrap .I need some suggestion ???

    html and css

    1
    Walter Sono•660
    @waltersonoPosted over 3 years ago

    Hi susu548

    Nice work

    To answer your questions: I suggest that you use media queries.

    I believe that one should understand first how to do without any framework because when it comes the time to use a framework it will be very ease and will know what framework to use, what are it strength and weakness.

    Hope this is helpfull, keep coding

    0
  • Walter Sono•660
    @waltersonoPosted over 3 years ago

    Hi alexvillhen

    Nice work

    Here are my suggestions:

    • Use the headings in order

      • You use a h2 but there is no h1, this causes confusion for screen readers users
      • You could have used h1 for "Join our community" and h2 for the other titles
      • You can see by the design that "Join our community" is bigger then the other titles

    • Avoid using "ems" for margin

      • This unit is relative to the current font-size, and the global font-size. It multiplies
      • Use rem instead

    • Add hover effect to the button

      • This will make the user experience better

    Ok alexvillhen, hope this is helpful.

    Keep working.

    1
  • Walter Sono•660
    @waltersonoPosted over 3 years ago

    Hi rks1995, nice work

    The design looks good,

    Aside from what Vanza said, here are my suggestions:

    • Use the correct README.md file

      • Use the template provided in the resources, people are going to look at your repository and they need to understand what is the project about

    • Give an alternative text to each image

      • This helps screen readers users understand your content

    • Use the correct HTML tag

      • If you have a header use the header tag
      • The main tag should not contain the header, nor a footer
      • Every web page should have an h1

    • Use the headings in order

      • I see a h3 but i do not see a h1 nor a h2
      • This causes confusion to screen readers users when browsing through your page

    • Add an hover effect to the buttons and links

    • Add links to your footer items

    Ok rks1995, hope this is helpful.

    3
  • Nicholas•80
    @nicholask98Submitted over 3 years ago

    Hi all,

    This is my first challenge I've done in a few months. Looking to get back into front end a bit and would love any feedback you have.

    Thanks!

    Site using Flexbox

    1
    Walter Sono•660
    @waltersonoPosted over 3 years ago

    Hi nicholask98

    This is very good, the design looks for both layouts (mobile and desktop)

    Here are my suggestions:

    • Give an alternative description for each image

      • Alternative descriptions helps screen readers users understand your content

    • Try to target your elements by class not by structure

      • If you target your elements by structure that may compromise your design if you were to change the layout even a little bit
      • Instead of targeting the button by ".section > a > .button" just write ".button". If it is a special type of button then give it another class ".button .button--special"

    • Use the correct HTML for each element

      • For the button use a button tag
      • If it is a link then style the link directly do not insert a paragraph tag inside of it
      • This is also helps people with screen readers understand your page and present your content correctly
      • Also helps you reduce the amount of HTML and CSS you write

    • Add and max-width for mobile and center it

      • It will make the mobile design not stretch and will look even better

    Ok, nicholask98

    That is it, keep the good work!

    Marked as helpful
    2
  • Walter Sono•660
    @waltersonoPosted over 3 years ago

    Hi salymk, good job

    To change the overlay color you can use:

    background-image: linear-gradient(rgba(color,opacity), rgba(same-color,opacity)), url(path/to/image);

    Hope this is helpful

    See you!

    Marked as helpful
    1
  • Walter Sono•660
    @waltersonoPosted over 3 years ago

    Hi deepchhatralia nice work

    About your request

    • Background-image: linear-gradient(rgba(color, opacity),rgba(color,opacity)), url(path/to/image)

    Here are few other suggestions:

    • Avoid using inline styling

      • Inline styles are bad practice because of the high level of priority they possess
      • Also because it will be hard to maintain

    • Learn pure CSS first

      • This will make you understand any CSS framework
      • I see that you only used bootstrap because of the columns, but now CSS have flex-box and grid layout which can help you build columns and fluid layouts easily

    • Use the Headings in sequence

      • In this case i do not think you should be using h3, because that is not a subtitle
      • Also you are missing an h2, if there is no h2 then it shouldn't be a h3 (is not a best practice)

    Keep up with the good work! See you

    0
  • Walter Sono•660
    @waltersonoPosted over 3 years ago

    Nice work avi3101

    Here are my suggestions:

    • Learn pure CSS first

      • If you can understand CSS well you will understand any framework very well

    • Use the "alt" attribute well

      • "ALT text is accessed by screen reader users to provide them with a text equivalent of images." - https://accessibility.psu.edu/images/imageshtml/

    Marked as helpful
    1