Skip to content
OverviewSolutions
17
Comments
61
Muhammad Shajjar
@muhammadshajjar

All comments

  • Klaudia•50
    @HaravedSubmitted about 3 years ago

    Let me know what u think about code, and if that's the corect way to achive this solution! :)

    Thanks in advance!

    Huddle-landing-page

    1
    Muhammad Shajjar•1,100
    @muhammadshajjarPosted about 3 years ago

    Hello, congrats on completing your FEM challenge, yep you approach the design as needed but at some viewports, your content is overflowing and some other suggestions that would help you with your design and solution

    • Background-image is repeating, use background-repeat:no-repeat;
    • Add max-width to a body or make a container class that wraps your all content, by adding it you can control your content from growing too much on larger viewports and save yourself from this situation
    • The logo should be in anchor a, logos are used to navigate on the home page in most the cases
    • Your images should have alt attribute but for decorative images, you should leave alt as blank alt="" and use role=" presentation" or aria-hidden=" true" to make sure all screen readers ignore those images. In your case, the illustration image is decorative.
    • To overcome your accessibility issue you would need to add some semantic markers to designate sections of the page as the header, navigation, main content, and footer e.g:

    <main>
<div class”wrapper”> 
</div>
</main>

    Apart form this very well done, keep up the great work

    Marked as helpful
    1
  • Muhammad Shajjar•1,100
    @muhammadshajjarPosted about 3 years ago

    Hi saykeed, Nice work, I think some minor changes make your solution looks more similar to design

    • Some changes on the body, you should use this color background-color: hsl(210deg 46% 95%); as background-color
    • make your content perfectly center this would help
    use body or make container class{
    min-height: 100vh;
    display: flex;
    justify-content: center;
    align-items: center;
    • instead of giving widths to grid-template-columns use 1fr which is handier to achieve design
    • use max-width:1110px on card container because card container is not 1440px, this disturbs your hole design
    • Give a look to report to overcome accessibility and HTML issues

    Hope it would help you

    Marked as helpful
    1
  • Muhammad Shajjar•1,100
    @muhammadshajjarPosted about 3 years ago

    Hey Arslan, congrats on completing this challenge, you did a great job

    some suggestions would be

    • By using width on your card cause horizontal scroll or your content gets cut on the window at some viewports, use max-width instead, and adopt the habit of not specify fixed heights and widths, it would create unexpected issues for you
    • Have a look at mix-blend-mode or background-blend-mode to put an overlay on your images as it is in the design. check out this I used background-blend-mode to put overlay, not sure about it worked 100%, but it works to some extent
    • Use one h1 per page, mainly for tilting the page
    • Use rem or em , don’t go with pixels, if you are writing scss make a function to convert pixel to rem, it would be handy
    Marked as helpful
    0
  • Muhammad Shajjar•1,100
    @muhammadshajjarPosted about 3 years ago

    Hi, you already get the valuable feedback, I want to add some more points

    • Don't use width on .huddle-introduction-content instead of use max-width, becasue it causes horizontal overflowing
    .huddle-introduction-content {
    display: flex;
    /* width: 40rem;  (max-width: 40rem)
    • Add max-width to a body or make a container class that wraps your all content, by adding it you can control your content from growing too much on larger viewports
    • The logo should be in anchor a, logos are used to navigate on the home page in most the cases
    • Links must have discernible text, [check out this link] (https://help.blackboard.com/Ally/Ally_for_Websites/Instructor_Editor/Improve_Content_Accessibility/Indiscernible_Link#:~:text=What%20does%20discernible%20text%20mean%3F%20Links%20generally%20have,looks%20like%20this%3A%20%3Ca%20href%3D%22%23best-practices%22%3EBest%20practices%20for%20links%3C%2Fa%3E)
    • use media queries to prevent the layout from breaking

    Aside from this Great effort, keep it up

    0
  • Rizqy Fachri•110
    @RizqyfmSubmitted about 3 years ago

    Hello again! This is my third solution for this community!

    As always, I tried to make 10/10 similarity between my solution and the design. I found the obstacles when making the social media logo on the footer, but I think I figured it out.

    Any feedback, advice, and suggestion is what I need :)

    Have a great day fellow developers!

    Landing page using CSS Flexbox for desktop and phone display

    2
    Muhammad Shajjar•1,100
    @muhammadshajjarPosted about 3 years ago

    Hi congrats on completing your third FEM solution, you succeeded in similarity, some suggestions that would help you

    • By giving width on illustration-Image, it causes a horizontal scrollbar at some viewports, use max-width instead
    • Add max-width to a body or make a container class that wraps your all content, by adding it you can control your content from growing too much on larger viewports
    • The logo should be in anchor a, logos are used to navigate on the home page in most the cases
    • always have a look at the report, it helps you to overcome accessibility and HTML issues

    Happy coding :)

    Marked as helpful
    0
  • Muhammad Shajjar•1,100
    @muhammadshajjarPosted about 3 years ago

    Hey, Great job on finishing the project! The layout looks decent, HTML and CSS are structured very well, some suggestions that would help to improve your solution more

    -Try to add max-width to a body or make a container class that wraps your all content, by adding it you can control your content from growing too much on larger viewports

    • Better to use blockquotes for testimonial quotes text instead of writing it directly in div
    • Font-size of top-section is a little bit off that make your solution off from design
    • Form validation and active states are missing, reffer to design/active-states.jpg
    • You can validate your form with pure CSS here is the link to CSS tricks
    • Footer get stuck with window at the bottom add some padding-bottom to it

    Apart from this very well done, keep up the great work

    0
  • Tomi•565
    @Tomi-pterSubmitted about 3 years ago

    Feedback will be appreciated!! 😁 I have a problem with the color of the social media icons when its hovered. I can solve this by importing editable icons from somewhere else but i'll like to use the assets contained in the starter pack. I'd appreciate any help.

    Clipboard-landing-page. html/css

    1
    Muhammad Shajjar•1,100
    @muhammadshajjarPosted about 3 years ago

    Hi Nice Work, Awesome work on this one layout looks great and responds well on all sizes but there is a small issue that content is growing too much on larger viewports add max-width to a body or make a container class that wraps your all content, by adding it you can control your content from growing too much on larger viewports, it is first stepped to responsiveness in my opinion.

    I think you are on the right path of using the filter to your image on hover but the filter you used is not right Check out this code-pen it will generate filter which you want This is generated by this code pen,

    .social a:hover {
filter: invert(56%) sepia(100%) saturate(338%) hue-rotate(122deg) brightness(90%) contrast(85%);
}

    And if you would use direct SVG in your Html, This css tricks link would lead you to the best

    Apart form this very well done, Keep up the great work

    Marked as helpful
    0
  • Muhammad Shajjar•1,100
    @muhammadshajjarPosted about 3 years ago

    Hi Mohamed! Awsome work layout looks good and HTML is very well structured

    some suggestions are

    • The logo should be in anchor a, logos are used to navigate on the home page in most of the cases
    • The register now should be an a link, in my opinion, using a tag and not a button. Use the button if it will act as a control for something. But on this, it is treated as a link, go for a tag.
    • Avoid too much class nesting

    hope it would help you

    Marked as helpful
    1
  • Muhammad Shajjar•1,100
    @muhammadshajjarPosted about 3 years ago

    Hi!

    To your question; Justify-content:centre and align-content: centre would place your grid-section-container to perfectly centre but you need to remove some styles check out this these are some styles that need to be removed

    Some other suggestions would be

    • Use only one h1 per page usually for tilting the page
    • Try to add max-width to your body or make a container class that wraps your all content, by adding it you can control your content from growing too much on larger viewports
    • Use blockquotes for quotes instead of using p
    • always give a look at report it helps you to overcome accessibility and html issues

    Apart from this very well done, keep up the great work

    Marked as helpful
    1
  • Basma•300
    @btebeSubmitted about 3 years ago

    I couldn't make the background look exactly as it was shown in the challenge. it would really be appreciated if I got some tips on how to do it or improve on my existing CSS. Thanks.

    Reactjs

    1
    Muhammad Shajjar•1,100
    @muhammadshajjarPosted about 3 years ago

    Hi congrats on completing your first FEM challenge, It looks good and behaves well on all viewports

    To your quesiton;

    Your background image is placed where it has to be, Use correct background color on body which is hsl(225deg 100% 94%) (very pale blue) And don’t need to use gradients on the background image, use as it is. Then your background looks like to the design You confused yourself in overlays, I have a little simple solution for you, Use background color in body and use background image in the body too here is my solution hope it would help you in this case

    Some other suggestions that would help you to improve your design

    • change should behave as it controls something, and for controlling I prefer to use button for navigating somewhere it is good to go with a
    • Aoid using width and height, you used width on your .card-container, using hard code would cause some issues, I know in this solution your design is not very much affected form this, but in future projects, this habit falls you in the cumbersome scenarios. Try max-width and min-height instead
    • You should need to add some semantic markers to designate sections of the page as the header, navigation, main content, and footer e.g:

    <main>
<section> 
</section>
</main>
<footer>
</footer>
    Marked as helpful
    1
  • Parham•200
    @frontendparhamSubmitted about 3 years ago

    Stats Preview Card Project: I will be glad to know your opinion about my project 😄

    HTML, CSS, FLEX-BOX

    1
    Muhammad Shajjar•1,100
    @muhammadshajjarPosted about 3 years ago

    Hi, Nice work! some suggestions would help you

    • By using width on your content and image cause horizontal scroll or your content is gets cut on the window at some viewports, use max-width instead, and adopt the habit of not specify fixed heights and widths, it would create unexpected issues for you.
    • It is highly recommended to read style-guide.md, In the style guide there are two font families, that would be better to use instead of using Ubuntu, font family highly affected your style if you want to get pixel perfect solutions.
    • Have a look at mix-blend-mode or background-blend-mode to put an overlay on your images as it is in the design
    0
  • Clarice Almeida•65
    @ClariceAlmeidaSubmitted about 3 years ago

    In my last challenge I was told to use more relative measure units so in this challenge I tried to focus on that. Please feel free to give me any tips of how I can improve the code.

    Responsive Layout

    1
    Muhammad Shajjar•1,100
    @muhammadshajjarPosted about 3 years ago

    Hi ClariceAlmeida, I’m glad to hear that you imply what you learn in the previous code reviews, keep doing this approach, You will surely succeed.

    Some feedback on your solution

    • change should behave as it controls something, and for controlling I prefer to use button for navigating somewhere it is good to go with a
    • Avoid using width and height, you used width on your card, using hard code would cause some issues, I know in this solution your design is not very much affected form this, but in future projects, this habit falls you in cumbersome scenario. Try max-width instead
    • Your images should have alt attribute but for decorative images, you should leave alt as blank alt="" and use role=" presentation" or aria-hidden=" true" to make sure all screen readers ignore those images. In your case, the music icon and your hero image are decorative.
    • background-image is little bit off, trying to fix it by giving proper size and position. Position top and size contain would work in your cause. Hope it would help you
    Marked as helpful
    1