Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found

All comments

  • @alekseibodeev

    Posted

    Hi, there 👋 Well done!

    Mobile navigation menu behaves a bit weird on collapsing. You can fix that by moving some properties from open state to general nav properties:

    nav {
    + position: fixed;
    + right: 0;
    + top: 0;
    + bottom: 0;
    + width: 75vw;
    + background-color: var(--off-white);
    + padding: 1.25rem;
      opacity: 0;
      z-index: -1;
      transition: all 0.3s ease-in-out;
    }
    
    .nav.open {
      oppacity: 1;
      z-index: 1;
    }
    
    0
  • @alekseibodeev

    Posted

    Hi @Meteogr03 👋 I don't have access to your project repository so it's hard to give detailed feedback.

    Here're my suggestions:

    • There is layout shift when submitting form with invalid data. You can fix that by alocating enough space for error messages before they actually appear on the screen.

    • Toast dialog doesn't fit on the screen. It's better to remove toast from main layout by giving it position: absolute or position: fixed.

    Hope that was somehow helpful 😌

    0
  • @alekseibodeev

    Posted

    Hi @TPAIN22 Great project!

    I would propose some UX improvements:

    • Add pointer cursor when hovering on accordion items
    • Add keyboard navigation support
    0
  • @alekseibodeev

    Posted

    Hi, there 👋 Card flip looks really awesome 🌟

    A little tip 💡:

    • Don't add tab index on non-interactive elements. Users definitely don't want to tab through all text on the page to reach something useful.

    Marked as helpful

    0
  • @chenmeister

    Submitted

    What are you most proud of, and what would you do differently next time?

    I am glad I have completed this project faster than anticipated.

    What challenges did you encounter, and how did you overcome them?

    I wanted to make the card element centered on all sides but had to use margin to do it.

    What specific areas of your project would you like help with?

    I was wondering if there is any way I can make the h2 and p elements appear next to each other so that the Name and Location items can look the same as the one in the solution. Also, if anyone has a more effective approach in getting the Card item centered on all sides that would be awesome. Any feedback is greatly appreciated.

    @alekseibodeev

    Posted

    Hi 👋 Nice solution!

    If I understand it correctly you want to reduce spacing between h2 and p elements. You can achieve it by reducing margins on those elements. For example:

    h2 {
      margin-bottom: 0;
    }
    
    p {
      margin-top: 0.25rem;
    }
    

    But it's better to replace type selector with class ones.

    To center card you can use the following code snippet:

    .container {
      min-height: 100vh;
      display: grid;
      place-content: center;
    }
    
    body {
      margin: 0; /* will remove unnecessary scroll bar */
    }
    

    Marked as helpful

    0
  • @alekseibodeev

    Posted

    Hi, there 👋 Nice solution!

    Here are some thing you can improve:

    Accessibility:

    • Ensure all elements have good contranst ratio. It's hard to read text when hovering on rating selection buttons.
    • Rating selection button are not accessible with keyboard only. That's because you use <div> elements which don't have tab index by default. It's more appropriate to use radio buttons here.

    React:

    • It's more "reactive" way to control your UI with state, but not with side-effect functions. Instead of touching classNames directly in handler you can use state to update UI. For example:
    const App = () => {
     const [isOpen, setIsOpen] = useState(false);
    
     const submitClickHandler = () => setIsOpen(true);
    
     return (
       <Wrapper>
         <Button onClick={submitClickHandler}>;
         <HiddenComponent className={isOpen ? "" : "hidden"} />
       </Wrapper>
     );
    }
    

    Marked as helpful

    0
  • @alekseibodeev

    Posted

    Hi 👋

    The solution looks nice visually, but navigation does not work. Consider share state between <ActivityInfo /> and <Profile /> components by lifting it up.

    Marked as helpful

    0
  • @alekseibodeev

    Posted

    Nice solution!

    Here are some things you can improve:

    Semantic input type

    HTML5 has a lot semantic input types with built-in constraint validation. You can use appropriate type to prevent user from entering non-numeric data.

    <input type="number" />
    

    You can style it as a text input with following code:

    input::-webkit-outer-spin-button,
    input::-webkit-inner-spin-button {
      -webkit-appearance: none;
    }
    
    input[type=number] {
      appearance: textfield;
      -moz-appearance: textfield;
    }
    

    Disabled state awareness

    By providing appropriate styles you can ensure that user understand what's happening on the screen. You can style cursor to give user an extra hint that button does not clickable:

    button[disabled] {
      cursor: not-allowed;
    }
    

    User feedback

    It's good to provide additional feedback to user when something off. For example, tip calculator does not really want to "eat" negative tip %, amount of people or bill. It would be nice to show custom error message as you did for zero value.

    Hope that was helpful.

    1
  • P
    j-hogben 290

    @j-hogben

    Submitted

    What are you most proud of, and what would you do differently next time?

    ~ First time using .json or dynamically populating and updating the DOM.

    ~ I'm finding grid increasingly comfortable to use, practise practise!

    ~ I was able to practise switch statements in there too.

    What challenges did you encounter, and how did you overcome them?

    ~ I found it challenging using the fetch API and functions to update the DOM.

    What specific areas of your project would you like help with?

    ~ Any advice on making the .json manipulation more efficient or more understandable for others reviewing code.

    ~ Any other advice or feedback is greatly appreciated!

    @alekseibodeev

    Posted

    Hi 👋 Perfect solution!

    If you want to improve readability of asynchronous code I would recommend to check async/await syntax out. async/await is syntactic sugar on top of promises that makes your code feels like a synchronous one. For example:

    Promises:

    fetch(url)
      .then((res) => res.json())
      .then((data) => console.log(data))
      .catch((err) => console.log(err));
    

    async/await:

    async function doStuff() {
      try {
        const res = await fetch(url);
        const data = await res.json();
        console.log(data);
      } catch(err) {
        console.log(err);
      }
    }
    
    doStuff();
    
    0
  • @alekseibodeev

    Posted

    Hi 👋 Nice solution!

    Here are some tips:

    HTML semantics

    It's better to use ul element for list like content. Your markup could look like this:

    <ul>
      <li>Product ... </li>
      <li>Measuring ...</li>
      <li>And much more</li>
    <ul>
    

    One extra advantage here except a correct markup is a bullet points customisation. You don't need to insert svg element before each item. You can change list style with this simple CSS:

    li::marker {
      content: url(./path/to/your/image.svg);
    }
    

    Modals

    There is a <dialog> element that makes it easier to work with modals. You can move success message to your markup and reduce JS code. Even though <dialog> relatively new it's well supported by all major browsers

    Error

    Your custom error does not work. That's because you're trying to use pseudo-element selector with self-closing element. When you use pseudo-element, browser will kinda nest it inside the original one. Example:

    CSS:

    div::before {
      content: /* something happening here */
    }
    
    .div::after {
      content: /* something happening here */
    }
    

    DOM:

    <div>
      ::before
      // div content
      ::after
    </div>
    

    But input element is self-closing:

    <input />
    

    That means there is no space for ::before or ::after element inside it because that "inside" does not exists

    Hope that was helpful. Keep coding.

    0
  • @alekseibodeev

    Posted

    Hi 👋 Nice solution!

    Here are somethings to improve:

    Mobile tooltip

    There is layout shift on mobile when toggling tooltip. This happends because you changingdisplay property on <figure> element and removing padding-bottom on .card. The easiset way to avoid those shift is to use positioning properties:

    @media (max-width: 600px) {
      .card .mobile-active {
        /* don't touch padding here */
      }
    
      .mobile-active figure {
        /* don't touch display property here */
      }
    
      .person-info {
        position: relative;
      }
    
      .mobile-active .share {
        position: absolute;
        inset: 0;
        /* width: 100%; you don't need this anymore */
        /* you other styles */
      }
    }
    

    Code snippet above will fix layout shift but you still have to adjust it a bit to look nice.

    Code style

    I would recomment to avoid using type selectors for specific styles. Use it only for resetting/normalizing CSS. For example, you have display: none applied to ul. What if you need another ul on the page? You will need to overwrite display property for each new one.

    JavaScript Variables

    For this small project this makes no difference, but it's good prectice to avoid var keyword since it can introduce bugs later on. Use const for all unchanging variables/refferences and let for anything else.

    I hope that was helpful. Bye!

    Marked as helpful

    0
  • P
    Daniele 300

    @dedo-dev

    Submitted

    What are you most proud of, and what would you do differently next time?

    Next time I will approach my work differently, I have to work more for what my skills are, without trying to rush things, for now it's just a waste of time.

    What challenges did you encounter, and how did you overcome them?

    The biggest challenge I have encountered has been myself, which I overcame by taking three breaths and moving forward to the best of my ability.

    What specific areas of your project would you like help with?

    A generic review and every kind of feedback are welcome 😎 Thanks ✌️

    @alekseibodeev

    Posted

    Hi, there 👋. Great solution!

    Here are some tips 💡

    Background-image

    You can place background image without bloating HTML with just decorative content and avoid relative positioning:

    .testimonial {
      background-color: /* your color */
      background-image: /* path to image */
      background-position: /* position relative to container */
      background-repeat: /* turn on/off repeat pattern */
      background-size: /* image size */
    }
    

    You can read more about background-* properties on MDN article

    BEM

    Since you use BEM it's good to make use of modifiers. Modifiers are more appropriate option for alternative style variants for your blocks. For example you could have something like this:

    HTML:

    <article class="testimonial testimonial_variant_darkblue">
      ...
    </article>
    

    CSS:

    .testimonial {
      /* default styles */
    }
    
    .testimonial_variant_darkblue {
      /* theme specific styles */
    }
    

    You still need to use .g-item-{order} selectors for positioning.

    Marked as helpful

    1