Skip to content
OverviewSolutions
13
Comments
13
P
Nitya Gulati
@nityagulati

All comments

  • Madi•45
    @madizhaksylykSubmitted over 4 years ago

    Hi everyone! I worked on atom and the final code looked just fine. But now the last box is not where it should be. I know, my code seems a bit long and messy, so I want your tips.

    solutionZMZ

    3
    Nitya Gulati•665
    @nityagulatiPosted over 4 years ago

    Hi Madi, nice work. The site looks good on desktop and all the cards are where they should be. Some pointers to further refine the code --

    • The card icons are not showing. You will have to update the src path for the images.

    • Here's what you can do to cut down on the code repetition. You can add a separate class such as card to all the cards and add all the common attributes under that and keep the unique styling under the individual card classes such as karma etc.

    <div class="card karma"> .card {...} .karma {...}

    • Have you looked into CSS Flex/Gridbox? If not, I would suggest giving that a try. It's very easy and convenient to use to create such layouts without having to use position. This also helps with making the site responsive, instead of adding positions for each breakpoint.

    • The next step would be to look into media queries to make it mobile-friendly. Also, you can look into the mobile-first approach as Ndoy3m4n suggested. You can code for the mobile breakpoint and then use min-width to scale it up. This helps cut down on the code and also has performance gains when loading the site on mobile.

    Keep up the good work!

    3
  • Sol B W•80
    @solybarrSubmitted over 4 years ago

    Hello, I used grid for the positioning of the desktop design. The only thing I couldn't do was get the image larger like I saw some other coders did.

    I had an accesibility error so I had to add a "label" element to my form even though it was empty. Its really good to run your html through HTML Codesniffer to get it validated.

    Any tips would be welcome. This is my first solution here and my first time using CSS Grid.

    Base apparel with Grid positioning for desktop

    4
    Nitya Gulati•665
    @nityagulatiPosted over 4 years ago

    Looks good, Sol! Your code is clean and well structured. Great work coding mobile-first and using Grid.

    • You don't need to add the space character inside the hero div. You can just have an empty div in the html and it will still take the background image and height/width settings from your css.

    • Currently, if the email field is empty, it displays the error for a few seconds and then submits the form. You will need to prevent the form from submitting when the email does not validate in your js code.

    • Mobile breakpoints 375px and below show a horizontal scrollbar, you probably need to adjust the margins slightly. You can test it out using Chrome Developer tools.

    Keep up the good work!

    1
  • Nitya Gulati•665
    @nityagulatiPosted over 4 years ago

    Hi Stowman, nice work and welcome to the community!

    Currently, the text is too big on desktop and too small on mobile. Just slight changes to font sizes and margins and padding would improve the design and make it closer to the design previews.

    I noticed that you used Bootstrap. Although that's completely fine and Bootstrap is a good tool to know and use in many projects, I would suggest not to use it since you are just starting out, especially on the newbie challenges. That way you can get a good foundation of working with html/css and building layouts on your own using Flex/Grid, which can be very beneficial. Once you feel like you have a good understanding of it you can use Bootstrap or other tools as you see fit to enhance your skillset.

    Keep up the good work! :)

    1
  • Nitya Gulati•665
    @nityagulatiPosted over 4 years ago

    Great work, Mihir. Nice touch with the ease-in transitions for the text!

    Please note, that currently your JS code is not being applied and the form is using default browser validations because you linked script.js in your HTML file, however your code resides under js/min.js

    1
  • Nitya Gulati•665
    @nityagulatiPosted over 4 years ago

    Sijan, the site looks great! Some pointers that can help simplify the code further.

    • button-details div -- Instead of using position: relative and multiple space characters in the html file, I would use Flex to space and align the elements.

    • Cards -- Instead of adding <p> and <div class="line"> elements, you can simply use <ul> and then add borders to the <li> tags.

    • As the HTML report suggests, unchecked = false isn't a valid attribute. When you want the checkbox to be checked, you can simply add the checked attribute to the input tag. And when that attribute is not present, it's false by default.

    <input type="checkbox" id="test" name="test" checked>

    <input type="checkbox" id="test" name="test">

    You can also look up HTML5 semantic tags and try it out in the next challenge. Love the slide effect to move the card down when the viewport is small.

    Keep up the good work! :)

    1
  • Saka Sheriff A.•30
    @sheriffsakaSubmitted over 4 years ago

    Please, I will appreciate any idea on how to improve on this solution and what to add or removed from it. I look forward to your suggestions. Thank you

    Easybankfiles

    2
    Nitya Gulati•665
    @nityagulatiPosted over 4 years ago

    Hi Saka, here are some suggestions to improve upon the code.

    • Go through the report generated for the solution and implement the fixes recommended.

    • Move all the inline styles you currently have in the HTML file to your CSS file. Inline styles make it more difficult to revise and modify the site and it's not the best way to go.

    • Instead of using float: left and position: relative etc and adding empty <p></p> and <br> tags to achieve the layout, you can use Flex or Grid to align and space the elements and then add appropriate margins and paddings as Richard suggested. Floats are fine to use in small cases such as positioning a button or a particular element. However, it's not the best practice to use them for creating layouts.

    • Instead of using <li1> and <li2> tags. I would simply use <li> tags and then use a class to apply different styles as needed. Also, <li> tags should always be child elements within the <ul></ul> or <ol></ol> tags.

    Keep up the good work! :)

    1
  • Nitya Gulati•665
    @nityagulatiPosted almost 5 years ago

    The page looks good, Archdron. The code is clean and readable and I like that you split up the stylesheets, nice work! Is there a reason that you put the border-color for the cards as inline style in your HTML? I would recommend moving that to your external css.

    Have you worked with HTML5 semantic tags before? That's something you can work with in your next project. It helps with screen readers and accessibility as well.

    And you can also look up SMACSS architecture for organizing your styles, I think you might like it :) I'm currently working with it and it definitely makes life easier especially for bigger projects.

    2
  • Oliver•20
    @oliversibinSubmitted almost 5 years ago

    FINISHED , I WILL LOVE THE FEEDBACK , I NEED TO WORK ON MOBILE VERSION ! :)

    HTML5, CSS3

    1
    Nitya Gulati•665
    @nityagulatiPosted almost 5 years ago

    Great work on the project, Oliver! The desktop version looks good. Next step would be to add media queries for the mobile version.

    Here's a few suggestions after quickly going through your code --

    • Instead of using position: relative and float to position the card elements, you should use Flexbox or Grid. You can check out the tutorials from Wes Bos What the Flexbox?!.

    • Look up CSS naming conventions such as BEM to use more descriptive class names instead of generic div1 div2 etc. This helps in maintaining and scaling your code easily as well as debugging.

    • You can also read up on HTML5 semantic tags for laying out your HTML structure.

    Happy Coding :)

    1
  • Nitya Gulati•665
    @nityagulatiPosted almost 5 years ago

    Nice work, Abhishek! The page looks good and is responsive. Few suggestions to improve upon the code --

    1. Instead of using <div> tags for the card details, you should use <ul> as they are a list of features.

    2. Use meaningful, descriptive class names to style the elements such as card-list or card-details instead of row.

    3. Instead of adding another div - extra-height and applying height to it, you can simply add padding-top and padding-bottom to the card-selected element.

    4. I believe you are overusing the display: flex property on some elements that don't necessarily require it. The main element that needs it is cards container that holds and aligns all three cards next to each other.

    • card class (applied to each card) doesn't require flex property because <div> elements are block elements and are naturally aligned in columnar/vertical orientation.
    • row doesn't require flex as you don't have any child elements within the rows that need to be aligned. Also if you use <ul> instead of row divs then they will be listed as a list (column) naturally.
    • you can remove all the accompanying flex properties from these elements as well. flex-direction align-content justify-content align-items.

    1. You can instead use text-align: center on card class to align the content within the cards.

    2. Removing Flex from card will resize the buttons. You can use width: -webkit-fill-available for the buttons to stretch or specify a width using % (responsive) or px.

    Keep up the good work!

    1
  • Nitya Gulati•665
    @nityagulatiPosted almost 5 years ago

    Great work, Dani! The page looks good on both desktop and mobile. One small issue though, the footer is showing inside the 'Karma' card on my laptop - screen width 1280px. Instead of using position: absolute, you can use margin or margin-top to place and space out the footer from the rest of the content.

    1
  • Nitya Gulati•665
    @nityagulatiPosted almost 5 years ago

    Hi Saurav, nice work...the site looks good on the laptop. If you want to go a bit further and get the page looking exactly like the design then you can add these few tweaks --

    • Add a box-shadow and hover state for the Register button
    • Currently on the laptop (1220px), the page shows a vertical scroll. To fix that you can resize the image and push the social media and footer sections up to fit everything in single screen frame similar to the design.

    Great job on using flex to switch to mobile layout, however there are some scaling issues at the moment.

    • The mobile design isn't scaled down properly. You'll probably have to go back in your media query and resize the elements to better fit the screen size.

    • Also your current media query for min-width 320px and max-width 480px covers most of the smartphone devices. But I would add couple more breakpoints (as needed) for iPad/tablet devices as well to resize everything accordingly.

    Few breakpoints to consider and test

    min-width: 576px //Bootstrap small device breakpoint (landscape phones)

    min-width: 768px //iPad devices

    min-width: 992px //Bootstrap large device breakpoint (desktops)

    min-width: 1024px //iPad Pro devices

    You can also check out this extensive list of popular device screen sizes

    If you are not using it already, Chrome Developer Tools is a quick and easy way to test and debug different screen breakpoints.

    Good luck! :)

    1
  • Roybrussel•35
    @RoybrusselSubmitted almost 5 years ago

    This is my first ever HTML + CSS challenge. Completed within 15 hours. I have no experience with building mobile sites yet.

    HTML + CSS first ever challenge

    3
    Nitya Gulati•665
    @nityagulatiPosted almost 5 years ago

    Hi Roy, the website looks great on desktop and tablets, nice work. Next steps would be to add CSS media queries and make the site responsive for mobile design.

    Also try working with Flexbox or Grid to create the layout instead of using absolute position, that will make switching the layout for mobile design easier.

    Another thing to consider, some of your class names are quite generic .div1, .div2 etc. Although perfectly fine for this project, it's a good practice to use more intuitive and meaningful names for your elements. You can look up CSS naming conventions for tips and ideas. How to name CSS classes

    Keep up the good work!

    1