@madizhaksylyk
Submitted
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.
Looking to hire developers?
@nityagulati
@madizhaksylyk
Submitted
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.
@nityagulati
Posted
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!
@solybarr
Submitted
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.
@nityagulati
Posted
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!
@stowman2
Submitted
I would love someone to tell me how I did on this. I am just starting out with HLML and web design.
@nityagulati
Posted
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! :)
@the-mihir
Submitted
Please, Need Some Input.
@nityagulati
Posted
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
@sijandahal
Submitted
As this is my first junior challenge, feedbacks are always appreciated ! Created a responsive site with HTML, CSS and JS .
Thank you !
@nityagulati
Posted
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! :)
@sheriffsaka
Submitted
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
@nityagulati
Posted
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! :)
@archdron
Submitted
Hi, so this is my take on the challenge as a first-timer in HTML + CSS. Can't say if it is elegant or whatever, but there surely are some points for improvement. Could you, please, point them out for me?
@nityagulati
Posted
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.
@oliversibin
Submitted
FINISHED , I WILL LOVE THE FEEDBACK , I NEED TO WORK ON MOBILE VERSION ! :)
@nityagulati
Posted
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 :)
@abhikulshrestha22
Submitted
How to toggle the price (annually or monthly) without JS. As toggle button and price are not being direct parents /child or sibling relationship.
@nityagulati
Posted
Nice work, Abhishek! The page looks good and is responsive. Few suggestions to improve upon the code --
Instead of using <div>
tags for the card details, you should use <ul>
as they are a list of features.
Use meaningful, descriptive class names to style the elements such as card-list
or card-details
instead of row
.
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.
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.flex-direction
align-content
justify-content
align-items
.You can instead use text-align: center
on card
class to align the content within the cards.
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!
@dagonmar183
Submitted
This is my second project in FrontMentor.io
Your feedback is welcome!
@nityagulati
Posted
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.
@sauravchamoli17
Submitted
I have used flexbox for the project and developed it in my laptop and it is smaller than the actual desktop design. Constructive criticism is most welcome!
@nityagulati
Posted
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 --
box-shadow
and hover
state for the Register buttonGreat 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! :)
@Roybrussel
Submitted
This is my first ever HTML + CSS challenge. Completed within 15 hours. I have no experience with building mobile sites yet.
@nityagulati
Posted
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!
@domkoder
Submitted
Please, guys, you can go through my code and check out my work. Please tell me what am doing wrong or a better way of doing it.
@nityagulati
Posted
Hi Scotrolex, the site looks great on both mobile and desktop and the code is clean. Great job on doing mobile-first design! I personally find it to be a better workflow.
Some pointers to clean up the code further:
#second-dv
should be #second-div
.current
.container #second-dv .box h3 { font-size: 20px; letter-spacing: 1px; }
fixed
.container #second-div .box h3 { font-size: 20px; letter-spacing: 1px; }
Move the .attribution
styles from index.html to your stylesheet styles.css instead. All styles should be in the external stylesheet.
.attribution { font-size: 11px; text-align: center; }
.attribution a { color: hsl(228, 45%, 44%); }
Try to use class to style the elements wherever you can instead of ID. It's good practice to use classes for styling and only use ID when needed. This helps in reducing your code because you can reuse the class if more than one element require the same style, now or at any time in the future. This helps to keep your code maintainable and scalable.
add another class to each .<div id="box">
such as .box-1
, .box-2
, and so on.
` <div id="second-div">
<div id="box-1" class="box box-1">
<h3>Supervisor</h3>
<p>Monitors activity to identify project roadblocks</p>
<img src="images/icon-supervisor.svg" alt="Supervisor" />
<div class="clear"></div>
</div>
<div class="test">
<div id="box-2" class="box box-2">
<h3>Team Builder</h3>
<p>Scans our talent network to create the optimal team for your project</p>
<img src="images/icon-team-builder.svg" alt="Team Builder" />
<div class="clear"></div>
</div>`
use .box
class for styles that need to be reused
.box { border-radius: 10px; margin-bottom: 30px; padding: 0 20px 0 20px; box-shadow: -2px 18px 20px 3px hsl(0, 0%, 91%); }
.box-1
, .box-2
, and so on for individual styles.box-1 { border-top: 5px solid hsl(180, 62%, 55%); }
.box-2 { border-top: 5px solid hsl(0, 78%, 62%); }
add classes to each element such as .intro-title
, .intro-desc
etc.
<div class="container"> <div id="first-div"> <h1 class="intro-title"> <span>Reliable, efficient delivery</span> <br /> Powered by Technology </h1> <p class="intro-desc"> Our Artificial Intelligence powered tools use millions of project data... </p> </div>
use .intro-title
to style the h1
element instead of .container #first-div h1
.intro-title { font-size: 24px; font-weight: 600; }
you can also read up on css selectors and selector specificity to learn more about it
#first-div
, .container
etc are fine for small webpages such as these but it's always a good practice to choose names that are readable, intuitive and self-explanatory. This not only helps with debugging but also when collaborating with others or when you come back to a project after a while and have to make modifications. <div class="main-container"> <div id="intro" class="intro-section"> <h1 class="intro-title"> <span class="intro-title__span>Reliable, efficient delivery</span> <br /> Powered by Technology </h1> <p class="intro-desc"> Our Artificial Intelligence powered tools use millions of project data... </p> </div>
font-size: 1.5rem;
and margin: 5%;
. This makes the code more responsive because things can scale up or down according to another value such as screen size or the parent element.Few things to consider next and read up on
And of course, practice, practice, practice! Happy Coding :)