Skip to content
OverviewSolutions
44
Comments
44
Nneoma Njoku
@SatellitePeace

All comments

  • Nneoma Njoku•1,210
    @SatellitePeace

    Hello @dogukan0055 congrats on completing this project

    Your project looks great on the big screen but it is not responsive

    • To make your work responsive you have to change the direction of your cards and ratings to column because it is not possible for them to fit in a small screen as a row`
    #comments{
display: flex;
flex-direction: column;
    justify-content: center;
    align-items: center;
}


@media (min-width:800px) {
#comments{

flex-direction: row;

}
}

    Also, do the same for your header section

    • Also using a mobile-first approach which I used in the example above makes it easier for you to create a responsive site

    • Lastly, your CSS is very disorganized which makes it difficult to navigate

    • Your CSS code should have an order like this

    • Google fonts

    • reset CSS

    • root

    • body/ html

    • general styles (for example you may decide that all your p will have the same color so instead of repeating the colour you can do something like this p{ color: #fff; })

    • nav

    • header-

    • main

    • section

    • footer

    • media queries

    I Hope This Helps

  • Nneoma Njoku•1,210
    @SatellitePeace

    `Hello @Fakemilaz your card looks nice

    • min-height specifies the minimum height the body of your content should have

    • adding min-height to your body tag ensures that the length of your entire content shows and if your content length is more than 100vh the user will be able to scroll to see the entire content

    • For you to center your card with flex you have to add a height or min-height, you also need to add the justify-content property in addition to the align-items property

    • this is to ensure that your card is centered both horizontally and vertically

    • Also add a margin to the body so that the card will have some space from the body in a smaller screen

    • like the example below

    
body {
    background: hsl(212, 45%, 89%);
    min-height: 100vh;
    display: flex;
    align-items: center;
    justify-content: center;
    margin: 2rem,
}
    • Also when you add a max-width you should also add a width of 100% so that in screen sizes below 375px your card will adjust and still look good

    .container { max-width: 375px; width:100% margin: 0 auto; }

    
Overall you used flex, and margins properly 

- Do not use px for font-size use rem or em units instead

I Hope This Helps
  • Nneoma Njoku•1,210
    @SatellitePeace

    Hello @gabilucuta nice job

    here is a tip on how to get the value from the custom input

    Since the custom is a form input what you want to get is the value of your input

    so you need to add an input event listener to it so that whatever you input you can get the value

    example

      customTipPercent.addEventListener("input", () => {
 console.log(  customTipPercent.value)// to see the value of the custom input
// the rest of your code goes here
}
````

I hope this helps
    Marked as helpful
  • Nneoma Njoku•1,210
    @SatellitePeace

    Hello @Deepanshu-5288 congrats on completing this project

    There are a number of ways you can import your JSON data to your js and use it without having to copy the whole file into your js

    The most straightforward is async/await

    where you use an asynchronous function to fetch the data and return a promise

    so do something like this

    const url = "data.json"

async function myimport(){
const response = await fetch(url);
const data = await response.json();
console.log(data)// do this to ensure that the data has been brought in

// The rest of your code
}
````

I hope this helps
  • Nneoma Njoku•1,210
    @SatellitePeace

    hello @01Chloe nice job i particularly like the tweaks you made to the project

    On screens > 960px the card looks ok, but on larger screen the contents of the cards are overflowing

    • So here is my suggestions instead of using grid throughout, you can use a combination of grid and flexbox

    • in your main container you can have two sections one for the cards and one for the user.

    Display the main container to flex and give the user section a flex-basis of 23%

    • Then you can place all your cards in the second section and use the grid to create columns

    EXAMPLE

    main {
  max-width: 1200px;
  width: 100%;
  display: flex;
  column-gap: 2rem;
  align-items: center;
  justify-content: center;
  margin-bottom: 1rem;
}

sectionOne{
  max-width: 20rem;
  width: 100%;
  flex-basis: 23%;
}

sectionTwo{
  display: grid;
  grid-template-columns: repeat(3, 1fr);
  gap: 1.5rem;
  place-items: center;
}

//the above is for desktop first if you want a mobile-first do 

sectionTwo{
  display: grid;
  grid-template-columns1fr;
  gap: 1.5rem;
  place-items: center;
}
 - this means you don't need a grid-template-row

    • Also avoid giving heights to your card container just give them padding and allow the content to determine the height, this will help prevent overflow

    • Finally, instead of using JS to change the background color of the active button you can use the CSS focus() pseudo class so

    button:focus{
   background-color: #444;
}

Then you can then use the active class only for the first button because it has to be on focus when the page loads

so

````
btnWeek.addEventListener('click', () => {
   boxWeek.forEach(month => {
         day.classList.remove('hide');
     });
});

btnMonth.addEventListener('click', () => {
     boxMonth.forEach(month => {
        day.classList.remove('hide');
     });
});
````

I hope this helps
    Marked as helpful
  • Nneoma Njoku•1,210
    @SatellitePeace

    Hello @AustinKing5

    Your code looks good except for the fact that it is not responsive

    *** to answer your question ***

    • A clean code is basically, a scalable, maintainable, readable, and easy-to-understand code

    • So yes writing a clean code means writing a code that others can read

    • clean code does not always mean shortcodes, if writing short code means you have to cut corners and use certain hacks then don't write shorter codes because it will make your code difficult to read, understand and maintain

    • However, if refactoring your code to be shorter follows best practices and does not involve hacks and cutting corners then by all means write shorter codes

    • I tried to see why your site is not responsive but I could not access your GitHub so either the link is incorrect or the repo is set to private

    I hope these explanations help

  • Nneoma Njoku•1,210
    @SatellitePeace

    Hello @wshena your work looks great on mobile here is a tip for creating small screen menus

    Instead of changing the heigth from negative to positive values why not set the menu container to display:none

    add a new class in the css and set it to display:block then use if/else statement to Target the menu and to toggle the menu

    Like this

    CSS
slider{
display:none
}
.slider.show{
display:block
}

JS
function openAndCloseNav(){	// 
mainNavbar.addEventListener('click', () => {
	slider.classList.toggle(show)
});
}

openAndCloseNav()

    If you want nice animations you can use transform property to achieve that instead of display:none

    Doing it this way reduces your code a bit and gives you the same result

    I hope this helps

    Marked as helpful
  • Nneoma Njoku•1,210
    @SatellitePeace

    @rain-riot nice work but here are some problems with your card and some tips on how to solve them

    Your cards are not showing fully in fact on smaller screens i could only see two cards to fix this try this

    body {
  width: 100%;
  min-height: 100vh;
  display: flex;
  flex-direction: column;
  justify-content: center;
  align-items: center;
  background-color: hsl(0, 0%, 95%);
}

````
so instead of adding a fixed height of 100vh, your min-height should be 100vh in this way your card will not be below 100vh and if need be it will grow more than 100vh thereby allowing users to view the entire content


- second on smaller screens when the cards become a single column they are packed together because you did not give them a margin so add a margin-bottom to your cards on a smaller screen alternatively you can use the flex gap property so

````
.card{
gap: 1rem;
margin-bottom : 2rem;
}
    • Lastly on hover the text of your button disappears and is only visible when the button text itself hovered. This makes for a bad user experience so do not separate the link hover from the button hover try this instead
    #column3 button:hover {
  background-color: hsl(179, 100%, 13%);
color: #fff;
  border: 2px solid hsl(0, 0%, 95%);
  transition: ease-in-out 0.3s;
}
    • lastly do not put links in a button you either use a link or a button, not both so remove the link from the button and place the text directly in the button. In this way, you will not have to target the button and its contents separately like you did example
        <button><a href="#">Learn More</a></button>`

- this should be

    <button>Learn More</button>
````

I hope this helps
    Marked as helpful
  • Nneoma Njoku•1,210
    @SatellitePeace

    Hello @AMANDEEPSINGHBHALLA nice work

    But is an observation and tip

    -the background color is meant to cover your entire body

    and for the grey color to cover the entire screen you have to remove the width and height you set to the mainf container so instead of

    .mainf{
  width: 80vw;
  height: 80vh;
  margin: 4rem auto;
  background-color: hsl(212, 45%, 89%);
  display: flex;
  align-items: center;
  justify-content: center;
}

- why not do this

.mainf{
  display: flex;
  align-items: center;
  justify-content: center;
}
- I removed the height, width,  margin, and background-color

body{
  margin: 4rem auto;
  background-color: hsl(212, 45%, 89%);
}

- added the margin and background to the body element
    • i believe this will make your project more visually appealing but other than that your work is neat
    Marked as helpful
  • Nneoma Njoku•1,210
    @SatellitePeace

    Hello @OgunremiZainab congrats on completing this challenge

    • to answer your questions

    • the light grey color is meant to cover the entire screen which means the light grey should be placed on the body element

    body{
background-color: lightgray;
}
    • The width property determines how wide the content of your project will be so the width property should be set to the container element and not the body
    main{
max-width: 375px;
width: 100%
}
````

- I recommend that you use mobile-first approach in most cases or all the time if possible.
mobile-first approach simply means designing the layout for the phone first and using media queries to change certain styles to suit bigger screens like tablets and desktops

- If you are using mobile-first approach, your media query will look like this

````
@ media screen and (min-width: 768px){
main{
max-width:700px;
width: 100%
}
}
````

- then if you are using a desktop-first approach your media query will have a max-width like so

````
@ media screen and (max-width: 375px){
main{
max-width:300px;
width: 100%
}
}
    • On the small screen your card looks great but on the large screen it does not, try setting the main container enclosing your contents to a max-width of 375px and a width of 100%, and also add a margin of 3rem to your body like so
    body{
margin: 3rem;
}

mainContentContainer{
max-width: 375px;
width: 100%;
}

    I hope this helps

    Marked as helpful
  • Nneoma Njoku•1,210
    @SatellitePeace

    Hello @DavidMartelCloutier your NFT card looks great

    • now to answer your question

    • There is a CSS property called z-index.

    • z-index determines the overlapping contents that will appear first without affecting the position or visibility of other contents

    • If you have not tried the z-index you should try it

    set the z-index of the icon to 3 then set the z-index of the image to either -1 or 1

    like so

    main__img:hover{
z-index: -1;
}

.icon__img:hover{
z-index:3;
}
````

I hope this helps
    Marked as helpful
  • Nneoma Njoku•1,210
    @SatellitePeace

    Hello @vid-szabi, congratulations on completing this challenge

    • here are a few tips Your card is not properly centered, to do this you can use
    Body{
display:flex;
Flex-direction:column;
Justify-content:center;
align-items: center:
min-height:100vh
}
    • Also set your main container to
    main{
display:flex;
Flex-direction:column;
Justify-content:center;
align-items: center;
}

    NOTE: your main container is the container enclosing all your elements

    Also the sample text of your card is still present you are meant to delete the text after using it to build the card

    Marked as helpful
  • Nneoma Njoku•1,210
    @SatellitePeace

    Hello, @Eleansphere nice work and congrats on completing this project.

    Based on your code the method you used in styling the button is the best way to style multiple buttons

    • I also noticed that you added the transition property under the code block with the pseudo-element for each car causing repetition of code
     .sedan > button:hover {
    background-color: hsl(31, 77%, 52%);
    color: white;
    transition: 0.4s ease;
  }

    instead of doing it this way you can just add the transition property to the general button styles like so

    button{
    transition: 0.4s ease;
//Other styles
}

    I hope this helps

    Marked as helpful
  • Nneoma Njoku•1,210
    @SatellitePeace

    Hello @ABQ4539 you did a great job

    to answer your question

    • min-width = minimum width this simply tells the browser that a particular element should not be less than a certain width for example
    main{
min-width: 200px
}

    This means the main container is allowed to grow beyond 200px but it is not allowed to shrink below 200px

    • so if your screen width becomes less than 200px, your main container will either overflow or become hidden if your overflow is set to hidden
    overflow: hidden;
    • max-width is the opposite of min-width
    main{
max-width: 375px
}

    This means that your container will not grow more than 375px but it can shrink below 375px

    • when using max-width, make sure you also set your width to 100% so
    main{
max-width:375px;
width:100%;
}

    This will ensure that when the screen width shrinks your content will still look good

    • I also noticed that in some cases you set your max-width to 100% like this
       .container {
        max-width: 100%;
        display: flex;
        flex-direction: row;
        justify-content: center;
        margin-bottom: 40px;
    }

    instead of doing this just set the width to 100% min-width can be 100% but max-width should not be 100%

    • Lastly do not use px to set font-size instead use relative units like rem this way when the user zooms in or out the structure of the site will not be distorted

    • in fact you should should relative units like rem and em more often than px

    I HOPE THIS HELPS

    Marked as helpful
  • Nneoma Njoku•1,210
    @SatellitePeace

    Hello @Joe-Whitehead and congrats on your return to web dev

    As for your questions I am assuming the frameworks you are talking about are css frameworks like scss, less, bootstrap, tailwind etc

    The framework you use all depends on how large the project is, how many pages, the project will have and how many sections the project will have

    Personally when dealing with large project I usually recommend a css preprocessor like scss or less

    Then if you are building a medium project or a prototype for a big project you can use a framework like bootstrap to speed things up

    But in all it is always best to use vanilla css and preprocessors like scss and less

    As for your second question, it is always best to visualise the project in your head try to build it in your head by so doing you will be able to figure out the key structures and how to organize your elements and which element to give a classname

    As you code there is always going to be a point where you will have to figure things out as you go, but thinking about the strategies to use before diving in makes your work easier and reduces the chances of you having to figure things out as you code

    I hope this helps

    Marked as helpful
  • Nneoma Njoku•1,210
    @SatellitePeace

    Hello @islaammhd you did a pretty good job

    However, the purple overlay is covering the photo

    You used rgba which Is one way of creating an overlay but there also another method called

    • mix-blend-mode

    So try this instead

    . cover {
mix-blend-mode: multiply;
opacity: 85;
}
    • you can play around with the opacity until you get the result you want

    I hope this helps

  • Nneoma Njoku•1,210
    @SatellitePeace

    Hello @rodriwa congrats on completing this challenge as for your question

    • You did a pretty good job in your case but here are two things that you might want to work on

    • It is better to place you media queries at the bottom of other styles because you only use the media queries to adjust certain styles to fit certain screen sizes, so placing it first can cause a bit of confusion to the person reading the code

    • try learning and using BEM style to name you classes BEM makes it easier for you to give your classes a decriptive name which will in turn make it easier for others to read and understand your css without having to read your html multiple times to know what certain classes are for

    • Lastly try using Html semantic element s like main, nav and section

    I hope this helps

    Marked as helpful
  • Nneoma Njoku•1,210
    @SatellitePeace

    Hello @chizoba-009

    Other than the fact that you did not do the form validation, Your projects looks very nice i especially like the slider

    • As for the form validation you can use bootstrap which will mean that you only have to do some if else statement in your JS like the example below

    • HTML

       <form>
            <div class="form-group col-6">
              <input type="text" class="form-control" id="email" placeholder="email@example.com">
              <div class="invalid-feedback" >
                Please input a valid email
              </div>
            </div>
            <button type="submit"">Get Started For Free</button>
          </aside>
        </form>
    • JS
    callToActionFormValidation();

function callToActionFormValidation() {
  form.addEventListener("submit", (e) => {
    const reg = /\S+@\S+\.\S+/;
    if (!reg.test(email.value)) {
      email.classList.add("is-invalid");
    } else {
      email.classList.remove("is-invalid");
      alert("email submitted successfuly");
      email.value = "";
    }
    e.preventDefault();
  });
}

    in this example, regular expression was used but you can use alternative means

    • Nice project and Happy coding
    Marked as helpful