Skip to content
OverviewSolutions
27
Comments
19
Santiago Moraga Caldera
@Remy349

All comments

  • Santiago Moraga Caldera•710
    @Remy349Posted 10 months ago

    Hola @jonathanlemus, excelente resultado. Me gusto mucho como quedo todo el diseño. Bien ahí.

    Solo una recomendación de mi parte:

    • Para mostrar todos los enlaces de GitHub, Frontend Mentor, etc... Te recomiendo que lo hagas usando una lista <ul> con elementos <li> y dentro de esos elementos <li> los elementos <a href=""> para mostrar los enlaces. Algo así:
    <ul>
    <li><a href="#">GitHub</a></li>
    <li><a href="#">Frontend Mentor</a></li>
    <li><a href="#">LinkedIn</a></li>
</ul>

    Esto sera de mucha ayuda para una buena semántica y estructura html y al SEO.

    Marked as helpful
    0
  • Santiago Moraga Caldera•710
    @Remy349Posted 11 months ago

    Hi @aazs-edu, congratulations for completing the challenge, the final result is amazing. I would just like to give you a couple of tips to improve your skills.

    • In the body, instead of using height: 100vh; it is preferable to use min-height: 100vh; to center elements.

      • With height you are giving a definitive height to the element, if the content needs more space it will overflow.

      • With min-height you are giving an element a minimum height, if you specify a lower height, the height that the element will have will be the value you assigned to min-height, min-height is not applied when the height is greater than min-height.

    • In mobile responsive design there comes a point where the design wants to break, this is because of the width: 340px; you use in your <div class="container"> tag, instead you can use a max-width: 340px; and in the <section> tag add a margin: 0 1rem; and remove the margin: 0 auto; you have in your <div class="container"> tag. This way you will have a better responsive design for mobiles.

    CONGRATULATIONS for completing the challenge, keep it up :)

    Marked as helpful
    1
  • Santiago Moraga Caldera•710
    @Remy349Posted 11 months ago

    Hi @Jonndev25, first of all, congratulations for completing the challenge, the final result is really amazing, you did very well. Honestly there is not much to comment you used the tags in a good way.

    If I had to say something it would be just a few tips:

    • If you want the card to be center aligned you can use place-content: center; instead of place-items: center; but this is just a matter of aesthetics. Your result is already good.

    • My other observation would be not to leave the alt in the img tag empty, mostly for SEO purposes.

    I hope this is helpful :)

    Marked as helpful
    0
  • Amr•710
    @amrmabdelazeemSubmitted 11 months ago

    The almost perfect design was not planned. x_x

    Since I stopped coding with React for a while I'm struggling a bit with hooks and components but that ain't something that would stop me though.

    Time Tracking Dashboard with ReactJS

    1
    Santiago Moraga Caldera•710
    @Remy349Posted 11 months ago

    Hi @amrmabdelazeem, many congratulations on completing the challenge, it is really amazing and the final design is great.

    I reviewed your React code and I want to give you a tip to refactor the code found in the App.jsx component. I cloned your repository and edited the code to change the logic you have when rendering the Activity.jsx component. Instead of doing those component renderings using the if else you have nested, I recommend this way:

    import React, { useState } from "react";
import Heading from "./Heading";
import Activity from "./Activity";
import data from "../data.json";

export default function App() {
  const [time, setTime] = useState("weekly");

  function handleTime(childTime) {
    setTime(childTime);
  }

  const getTimeframeData = (item, timeframe) => ({
    key: timeframe,
    current: item.timeframes[timeframe].current,
    previous: item.timeframes[timeframe].previous,
  });

  return (
    <div className="container">
      <Heading onHandle={handleTime} />
      {data.map((item, index) => {
        const timeframeData = getTimeframeData(item, time);

        return (
          <Activity
            key={index}
            id={index}
            title={item.title}
            current={timeframeData.current}
            previous={timeframeData.previous}
          />
        );
      })}
    </div>
  );
}

    A little explanation:

    Function getTimeframeData:

    const getTimeframeData = (item, timeframe) => ({
    key: timeframe,
    current: item.timeframes[timeframe].current,
    previous: item.timeframes[timeframe].previous,
  });
    • This function takes two arguments: item (representing an element of the array data.json) and timeframe (representing the current time period).
    • Returns an object with the keys key, current and previous, which correspond to the specific properties of the time period within the item.

    Component Rendering:

    return (
    <div className="container">
      <Heading onHandle={handleTime} />
      {data.map((item, index) => {
        const timeframeData = getTimeframeData(item, time);

        return (
          <Activity
            key={index}
            id={index}
            title={item.title}
            current={timeframeData.current}
            previous={timeframeData.previous}
          />
        );
      })}
    </div>
  );
    • Then, a mapping is performed on the array data using the map method.
    • For each item element, getTimeframeData is called to obtain the data specific to the current time period (time) and an <Activity /> component is rendered with that data.

    In short, the code uses the time state to track the current time period and a getTimeframeData function to get the time period specific data for each activity in the data array. Then, the interface is rendered based on this data.

    CONGRATULATIONS ON COMPLETING THE CHALLENGE. I hope my advice will be of great help to you :)

    Marked as helpful
    0
  • Santiago Moraga Caldera•710
    @Remy349Posted 12 months ago

    Congratulations, it is an incredible result. Just a couple of recommendations for you to consider:

    • Modify a couple of styles in the css, in the responsive design for phones comes a point where it does not adapt very well, this is because of the min-width: 420px you have in the body, remove it :)

    • There are many ways to center an element, one of them is the following and consider using in your <main class="page"> tag:

    /* USING FLEX */

.page {
  display: flex;
  justify-content: center;
  align-items: center;
  min-height: 100vh;
}

/* USING GRID */
.page {
  display: grid;
  place-content: center;
  min-height: 100vh;
}

    I recommend that you use min-height instead of just height.

    • In your react code I reviewed it and it is very well done, although there are many things you can improve, such as creating a component for the thank you card, separate from the component you have in the card component.

    I hope this will help you to continue improving your learning.

    Keep it up :)

    Marked as helpful
    0
  • Santiago Moraga Caldera•710
    @Remy349Posted over 1 year ago

    The result of your project is amazing, very well done. I will only make some recommendations with the use of HTML tags.

    I saw your html and you use a lot of <div> tags, in the end they helped you to complete the project and it looks very good, but there are more you could use. An example would be the <article> tags for the testimonial cards, but in this case it would be much better to use the <figure> tag and inside it use <figcaption> and <blockquote>.

    I leave this link for you to investigate a little more the use of these tags:

    Figure tag - HTML

    Well done, keep it up :)

    0
  • Santiago Moraga Caldera•710
    @Remy349Posted over 1 year ago

    Hi @Karrar-Hussein, the result you achieved in the challenge is excellent, very well done.

    Just a little tip to improve your structure when writing HTML and get a semantic improvement for your future projects and challenges:

    • Note that you used many <div> tags, which in the end led you to the desired result but my recommendation is that you try using other tags such as the <main> tag for all the main content of the page and the <article> tag for the cards, specifically the ones with the <div class="card banner"> classes.

    This will improve the way you write HTML and the semantics of the pages, I leave these links for you to learn more about its use.

    HTML <main>

    HTML <article>

    Keep learning :)

    Marked as helpful
    0
  • Santiago Moraga Caldera•710
    @Remy349Posted over 1 year ago

    Hi @Xjanus12x, congratulations for completing the challenge, the truth is that it is a very good result you achieved. I would just like to give you some tips for you to consider for your future projects:

    • Looking at the HTML you wrote I noticed that you used a lot of <figure> tags, for this project I recommend you to use the <article> tag, I leave you this link to learn more about it. You will have a better structure when writing HTML: HTML: <article>

    • This one is for the cards, there is a point where the cards don't fit well in the height, do the test trying to reduce the width of the window and you will see what I am talking about, you can solve this by adding a height to your .card__hours element:

    .card__hours {
  min-height: 100%;
}

    That should be enough to solve that little problem.

    • This tip is for your styles.scss file, use @forward instead of the @import. I leave you this link: SASS @forward

    I hope my advice will be helpful. Again congratulations for completing the challenge, excellent result. Keep learning :)

    Marked as helpful
    0
  • aisi•10
    @aisyeloSubmitted over 1 year ago

    I found this one hard to position below the card:

     <div class="attribution">
        Challenge by <a href="https://www.frontendmentor.io?ref=challenge" target="_blank">Frontend Mentor</a>. 
       Coded by <a href="#">Aisi</a>.
   </div>

    I styled it this way, however it's layout changes as the screen resizes:

    .attribution{
    display: flexbox;
    align-items: center;
    position: absolute;
    right: 0;
    left: 0;
    bottom: 10;
    padding: 0.5rem;
    text-align: center;
}

    I would like to ask what are the proper ways on how to fix it. Thank you.

    QR Code Component using HTML & CSS

    2
    Santiago Moraga Caldera•710
    @Remy349Posted over 1 year ago

    Hi @aisyelo, great job completing the challenge, a very good result!!!!!

    Just a few tips for you to take into consideration for future projects:

    To solve your problem you can center all the elements using "display: flex" in the "body", remove all the padding and margin you added, you can also remove the "position: relative" and "position: absolute" so it doesn't interfere in the styles. Now just add these styles to the "body"....

    body {
  display: flex;
  flex-direction: column;
  align-items: center;
  justify-content: center;
  min-height: 100vh;
}

    This should be enough to fix your problem.

    Again, congratulations on completing the challenge, keep learning :)

    Marked as helpful
    2
  • Santiago Moraga Caldera•710
    @Remy349Posted almost 2 years ago

    Hi @Mouradis, I like the final result of your project, it is very well done, just some tips for you to take into consideration for future projects:

    1.If you want to center an element you can do it very easily using Flex, what you did is fine, in the end we are different and we have different thoughts and ideas, but I want to show you this way to do it if you didn't know it yet:

    body {
  min-height: 100vh;
  display: flex;
  justify-content: center;
  align-items: center;
}

    With these simple lines you can center an HTML element, I applied it in the body as it is very practical and there is only one HTML element that you want to center, but in general it will depend on what you want to center and also the amount of elements to center.

    2.Note that you did not put the background image that the project should have, in order to do this there are different ways to do it, I will show you one that will be related to the portion of code that I left you in the first tip:

    body {
  min-height: 100vh;
  display: flex;
  justify-content: center;
  align-items: center;
  position: relative;
}

body::before {
  content: '';
  position: absolute;
  background-image: url(Your image here!);
  width: 100%;
  // This value may vary, it will depend on you
  height: 100%;
  // You can use this for the image position,
  // these values will depend on you
  top: 0;
  left: 0;
  // This is another way, these values will depend on you
  background-position: top 0 left 0;
  z-index: -10;
  // From time to time you will find it necessary to use the following values,
  // so that the image is not distorted
  background-size: Here you can use cover or contain, there are more;
  background-repeat: Here you can use no-repeat, there are more;
}

    This should be enough to add the background image, but as I said, there are many ways to achieve the same result, this is one.

    I recommend you to investigate more about these concepts, specially the pseudoelements (::before, ::after, .....), also try to find other ways to do the same, it will help you to improve a lot and to know different ways to solve these problems.

    I hope my advice will be helpful for your future projects, keep on learning web development :)

    0
  • Mgamboa•220
    @Marianellag1Submitted almost 2 years ago

    Hello!

    Challenging challenge! I feel as though I am learning quite a lot, but always there are things I need to improve. Am I using the correct tags? Is there anything I need to keep in mind to simplify things? Does it look like I'm over thinking things (which I do)? Thank you!

    Testimonials Grid made with CSS

    1
    Santiago Moraga Caldera•710
    @Remy349Posted almost 2 years ago

    I saw the final result of your project and it is amazing, love it. Just a little tip for you to improve when using tags in HTML. Note that you used a lot of <div> tags which in the end gave you the expected result but there are many more tags you can use, like <main>, <article>, <figure>, <figcaption>, <blockquote> tags. Personally I used the following structure:

    <main>
  <figure>
    <div>
      <img />
      <figcaption>Text here!</figcaption>
    </div>
    <p>Text here!</p>
    <blockquote>
      " Text here! "
    </blockquote>
  </figure>
</main>

    I leave you this link so you can learn a little more about these tags and apply this new knowledge to your future projects, keep learning about web development

    Just one more thing, keep in mind the responsive design to make your projects adaptable to all devices, I recommend you to use the Mobile First design methodology.

    Figure tag - HTML

    And don't worry about overthinking things, it's okay to be curious and ask questions, it will clear your doubts and you will learn more :)

    Marked as helpful
    0
  • Santiago Moraga Caldera•710
    @Remy349Posted almost 2 years ago

    Excellent work, just a few tips for you to improve:

    1. This one is for the colored line that is at the top of the card, note that you used a <div> to achieve this result and that's fine but I want to introduce you to one that I know, you can add a "position: relative" to the <article> tag that you used to made the card and then this to get the same result:
    .article::after {
  content: '';
  position: absolute;
  top: 0;
  width: 100%;
  height: whatever you want, I used 5px;
  background: color here;
}

    this will give you the same result try it or keep it in mind for future projects :)

    2.The social network icons do not appear, instead only the colors appear but not the icon, you can fix this by using "color: icon color;" this to the <i> tag you used to put the icons.

    3.Note that you used sass for the styles, try using @forward instead of @import to import your scss files, I recommend you to read a bit more about it.

    I hope you find my advice helpful and keep on with your web development :)

    Marked as helpful
    0