Skip to content
OverviewSolutions
28
Comments
8
P
Matthias Nilsson
@RalfiSlask

All comments

  • Matthias Nilsson•980
    @RalfiSlaskPosted about 1 year ago

    Hello!

    Regarding star image

    Easiest way to fix the issue is to have the star image inside of a div and then center the image using flexbox. That way by seperating them it is much easier to not get any unwanted behaviours.

    1
  • Matthias Nilsson•980
    @RalfiSlaskPosted about 1 year ago

    Hello!

    Javascript feedback

    Variable Names

    Change the name of btn to buttons as it targets all the buttons and not a single one, dont be afraid of writing larger names as it is important for other programmers to be able to understand your names. For example bck can be really hard to understand what it means.

    Selectors

    Have all the selectors at the top of your file.

    Using var instead of let and const

    I would suggest using the more common standard let and const for declaring variables as var is a bit outdated. It is generally better to use let and const instead of var because of their nature in scoping and hoisting which can lead to some bugs if you are not familiar with its nature. Also how you declare your functions changes the hoisting, arrow functions will not be hoisted where function() will be.

    Let and const are block scoped and var is function scoped, var will also be hoisted which means that it bubbles up to the top of the file. Block scoped means that you can only handle the variables inside blocks for example if else where function scopes means that you can reach variables everywhere inside a function.

    const checkScope = () => {
  console.log(varInsideBlock) // will print undefined 
  cause of hoisting this becomes the declaration var varInside; 

  if(1 === 1) {
    var varInsideBlock = "function scoped";
    let letOrConstInsideBlock = "block scoped";
  };

  console.log(varInsideBlock) // will print 
  console.log(letOrConstInsideBlock ) // will produce ReferenceError
};

checkScope();

    Array methods

    Use array methods instead of for loops when you are working with arrays or NodeLists. For loops are better if you are working with numbers etc.

    buttons.forEach(button => {
  button.addEventListener("click", function() {
    button.classList.toggle("selected");
    result = button.innerHTML;
    span.innerHTML = result;
  })
  button.addEventListener("blur", function() {
    button.classList.remove("selected")
  })
});`
    Marked as helpful
    0
  • Matthias Nilsson•980
    @RalfiSlaskPosted about 1 year ago

    Hello nice work!

    I have a few suggestions and tips.

    HTML

    Change semantics of your elements so instead of <div className="footer></div> use <footer></footer>. Use only section if it is necessary.

    Javascript

    Maybe use ternary operators for simple if statements, it is easier to read as it only takes up one line.

    instead of:

    if (isDiscounted) {
     newPrice = newPrice * 0.75;
 }

    use:

    newPrice = isDiscounted ? newPrice * 0.75 : newPrice

    React

    Make use of react by creating components that can be reused if necessary so you dont have to repeat code. The most obvious example is in the footer, where you have three items that could be used in a single component.

    If you instead create a component like this and make use of the text as a prop:

    const FooterFeatureItem = ( {text} ) => {
  return (
    <section className="check-section">
        <img src="./icon-check.svg" alt="" />
        <h3>{text}</h3>
    </section>
  )
}

export default FooterFeatureItem

    When you then have this component you can either use it by just writing it like this:

    <section className="bottom-section">
       <FooterFeatureItem text="Unlimited websites"/>
       <FooterFeatureItem text="100% data ownership"/>
       <FooterFeatureItem text="Email reports"/>
 </section>

    But a better and more dynamic way is by first creating an array of objects:

    
const footerFeaturesList = [
        {id: 1, text: "Unlimited websites"}, 
        {id: 2, text: "100% data ownership"}, 
        {id: 3, text: "Email reports"}
]

    And then use the map method over this list to create the components:

     <section className="bottom-section">
     {footerFeaturesList.map(feature => <FooterFeatureItem key={feature.id} 
     text{feature.text}/>)}
 </section>
    Marked as helpful
    1
  • Prajwal Tomar•150
    @prajwal-tomarSubmitted about 1 year ago

    I used absolute positioning in order to position the card images. not sure if it is the right way.

    Tailwind CSS + React

    1
    Matthias Nilsson•980
    @RalfiSlaskPosted about 1 year ago

    Hello!

    Possible Improvments

    Styling

    • Maybe remove spellcheck for the inputs.
    • Have hover and focus effects on both the inputs and the button.
    • Inputs should not be filled in from the start, only have the placeholders.
    • Have better spacing between the inputs, button. Tip is to use flexboxs gap!

    JS/Functionality

    • Add error handeling when the inputs have not been filled in, are strings instead of numbers etc. You can use regular expressions or other ways of checking this.
    • The confirm button should handle the functionality, you can for example create a list as a state that keeps track of all the input values in each input.

    React

    • Organize your code into smaller components instead of having everything in the App component, that way if you want to change the application and add for instance another input you can reuse the input component etc. Then in those component pass down props.
    Marked as helpful
    0
  • Matthias Nilsson•980
    @RalfiSlaskPosted about 1 year ago

    Hello!

    Mainly i would suggestion having a better file and folder structure which in turn will make it easier for both you and pleople looking through your code.

    First add the fonts to its own folder, for example assets/fonts/overpass.

    Second i would recommend to use external css stylesheets and javascript files as it is easier for the browser to read and also for others reading through your code and this is a more common standard then having everything in a bloated html file.

    Maybe adding some hover effect on the buttons would add a nice touch to the project aswell, dont know if that is in the figma description.

    Lastly, create your own readme file and take a screenshot of your project (firefox) and add to your Github profile. Instructions should be in the original readme.

    0
  • boris2912•270
    @boris2912Submitted over 1 year ago

    Hi everyone!!

    this is my first time i use 3rd part API in a project let me know if i make it correctly!!

    would really appriciate any type of feedback!!

    thak you!!!

    Advice generator app

    1
    Matthias Nilsson•980
    @RalfiSlaskPosted over 1 year ago

    Hello!

    Here are some things i think you can improve in your project:

    CSS:

    • Would suggest having the height 100% in body, html and main so that your background color covers the whole page.

    • Have spacing between your selectors so it is easier to read.

    JS:

    • Instead of using an XMLHttpRequest try using the newer more modern way using fetch.

    • Object deconstructuring. Instead of having to type out each name like data.slip.advice // data.slip.id you can write as const { advice, id } = data.slip which is the same as creating two variables const advice = data.slip.advice and const id = data.slip.id.

    • Template literals. Instead of writing text.textContent = '"' + advice.slip.advice + '"';. You can use `` and inside these you can type your variables using ${variable}.

        try {
       const response = await fetch("https://api.adviceslip.com/advice")
       const data = await response.json();
       const { advice, id } = data.slip;  
       text.textContent = `"${advice}"`; 
       adviceId.textContent = `ADVICE # ${id}`;
    } catch (error) {
        console.log(error)
    }
}

fetchData();

button.addEventListener("click", fetchData)
    Marked as helpful
    0
  • d8701a•240
    @d8701aSubmitted over 1 year ago

    If a user doesn't choose any value 1-5 and still hits submit, is there a code to prevent them from doing so?

    I tried to write javascript code where it would be impossible for a user to hit submit button if they don't choose any value 1 - 5 to give rating.

    In that case, they wouldn't be able to proceed even if they pressed submit, because there would be an alert displayed, warning them to first choose a value between 1 and 5.

    I know there should be an if statement and compare rating to the one we get from buttons as input, but I couldn't complete the code. The furthest I got was to be able to display this alert, but it was then displayed each time I pressed submit, instead of only when I didn't choose any value.

    Interactive rating component

    1
    Matthias Nilsson•980
    @RalfiSlaskPosted over 1 year ago

    You should probably make use of a boolean variable.

    1. First declare lets say buttonPressed = false.
    2. Change it to true in your click event listener for the buttons.
    3. Create an if else statement in the click event for the submit button.
    4. In the if-statement write if(buttonPressed === true) or if(buttonPressed) which is the same and in the if block write out your code.
    5. In the else statement you can either apply an alert (this is not very pretty though) or attach lets say a red error message on the container that lets the user know they need to press one of the buttons.

    Good Luck!

    2
  • Matthias Nilsson•980
    @RalfiSlaskPosted almost 2 years ago

    I think i had the same problem as you.

    It is that the browser wont let you use mixed content/having http and https requests.

    So if you change the http one to https i think it should work for you!

    Marked as helpful
    1