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

  • Alex Kim 1,325

    @alex-kim-dev

    Posted

    About that overflow / hamburger menu issue, right now you're applying overflow-x: hidden; to every element using universal selector. This is kind of radical approach, some elements might still require the default overflow to be displayed properly. I think we should be more precise with that. The navigation menu has position: absolute, and the closest positioned ancestor element is .container - so the menu is positioned relative to the .container. Applying overflow-x: hidden to this element will fix both issues. (And don't forget to remove overflow from everything).

    Great job on this challenge 🎉

    Marked as helpful

    1
  • Alex Kim 1,325

    @alex-kim-dev

    Posted

    Hi Daniel, good job on this challenge! I've been looking through it and here's what I want to mention:

    • it's not necessary to wrap every image in a <figure> element, it's required when you need to add a caption to it:
    <figure>
        <img src="elephant.jpg" alt="Elephant at sunset">
        <figcaption>An elephant at sunset</figcaption>
    </figure>
    
    • the alt attribute with description must be present on images, it helps screen reader users and displays on the screen if an image isn't loaded
    • the margins of body and main elements collapsing, resulting in weird body positioning (with an offset on the top). There's a great post on the topic
    • the <main> element already has semantics, role="main" is not necessary
    • use either <a> or <button>, but don't mix both like <a role="button">

    Good luck!

    Marked as helpful

    1
  • Alex Kim 1,325

    @alex-kim-dev

    Posted

    Hey Tamera, the cards resize vertically because the height of their content changes, as you resize the window. It's possible to make them of the same height if we apply align-items: stretch to their flex container, but then we should make some other changes:

    /* 750px+ */
    .main-component {
      margin: auto;
      display: flex;
      flex-direction: row; /* row is the default value, can remove this */
      justify-content: center; /* doesn't do anything in this case */
      align-items: stretch; /* change center to stretch, it's the default, so we can remove it too */
      max-width: 920px;
    }
    
    .main-component {
      margin: 0 1.563em;
      height: 100vh; /* to remove, we want the height of our cards to be auto */
    }
    

    The cards are ready, now we need to center the whole component on the screen

    body {
      height: 100vh; /* make body full screen */
      display: flex; /* now the .main-component auto centers, because it has margin: auto */
      margin: 0; /* remove the default margin so we don't get vertical scroll */
    }
    

    That's it, good luck & have fun on the next challenge!

    Marked as helpful

    0
  • P
    tediko 6,580

    @tediko

    Submitted

    Hello👋!

    This is an invoicing application build with ReactJS and styled-components. The application is used to manage invoices and allows the user to create, read, update, filter by status and delete invoices. There is an option in the app to switch between a dark and a light theme. All transitions are smoothly displayed by using Framer Motion library to create animations. It was by far the largest and most comprehensive project I have done so far. It showed me how important it is to plan so that you don't have to change things that previously worked well in the middle of the project. A valuable lesson!

    • The first time I used useReducer hook to manage application state. I noticed that my state logic getting more complex as the few elements of my state relies on the value of another element of my state. Read More
    • Together with useReducer, the useContext hook turned out to be handy. It allowed me to create common data that can be accessed throughout the component hierarchy without passing the props down manually to each level which in turn allowed me to avoid Prop drilling (also called "threading") that is the process you have to go through to get data to parts of the React Component tree. Read More(1). Read More(2)
    • In order to create a theme switcher and provide colors for components I used styled-components <ThemeProvider> wrapper component. By leveraging the theme provider I only need to write my styles in one single object and invoke them in any component that is a descendant of that provider. Read More
    • When creating the form I learned what Controlled Component is. In HTML, form elements such as <input>, <textarea>, and <select> typically maintain their own state and update it based on user input. In React, mutable state is typically kept in the state property of components, and only updated with setState(). Then the React component that renders a form also controls what happens in that form on subsequent user input. Read More
    • To make application more ADA compliant (which means the website should be entirely accessible using just keyboard) I prevent focus go outside the modal once the modal is opened. In this case, the focus trap turns on when the form or modal with invoice deletion/status change is opened. In order to create an accessible modal I followed this great tutorial that follow the WAI-ARIA Practices.
    • To animate the pages transitions and modals I used Framer Motion API. Framer Motion is an open source, production-ready library that's designed for creating creative animations. In order to support users who have enabled their device’s Reduced Motion setting and make accessible animations I used useReducedMotion hook. Based on whether useReducedMotion returns true or not we're passing different values to animate. That replace position transitions with opacity. Read More(1). Read More(2)
    • Handle 404 routes in React Router and provide a fallback component for displaying an imfamous 404 Page Not Found error to the user. Try to enter a page that doesn't exist - like 'invoice-tediko.netlify.app/gotcha'

    You can find more about the things I used in the project in the README on github. I just wanted to point out most important things here.

    Questions:

    1. As I am changing the theme and many colors are changing, I wasn't sure how to make the color system. I end up with object for dark/light theme with colors of each element and a few colors that are common. How would you approach that?
    2. I didn't want to create a special component for my headings since they have no logic. I created helper utility called typographyStyles where I put all styles that I use across application. Not sure about that tho.
    3. I am curious what method you use to name your components. I personally stick to option #2 because it has the most pros for me and a little longer import doesn't bother me.

    Bugs:

    1. Clicking twice quickly on header logo (navigate to home page, which render Invoices component) causes component not to render. I am convinced that this is related to Framer Motion and AnimatePresence that allows components to animate out when they're removed from the React tree. I think build in isPresent state when clicking twice quickly didn't change the state. I overcame this problem with simple event.preventDefault() with a setTimeout function but..

    Additional feedback or a criticism will be appreciated Thanks! 😁!

    Alex Kim 1,325

    @alex-kim-dev

    Posted

    Awesome job, Tediko! I see you put a lot of effort into this project. I like that everything is well organized, files, code, commits, it's really easy to navigate through and find out how things are connected to each other. And great job on writing readme! Here is some feedback:

    • animation feels slow, I would adjust duration to be around 200-300 ms
    • accessing local storage might throw errors in some cases, for example if a user turned off all cookies in Chrome, some older browsers might throw even if only third-party cookies are blocked. So It's a good practice to wrap it in a try-catch block
    • there are a few places with a lot of nested if-else statements, like in formValidation.js. That's increasing cyclomatic complexity and makes code hard to understand, especially for people not familiar with the codebase. Try to avoid nesting conditions. Ideally it should look like this:
    () => {
      if (conditionA) return value1;
      // the following code will be executed only if(!conditionA)
      if (conditionB) return value2;
      // the following code will be executed only if(!conditionA && !conditionB)
      return value4;
    }
    
    • would be nice to work a bit on loading performance. I had DOMContentLoaded: 1.99s on fast connection and 5.32s on simulated "fast 3G".

    Good luck and until the next awesome project!

    Marked as helpful

    3
  • @die-lowenkonigin

    Submitted

    Hi all, please checkout my submitted solution. It will be a pleasure to know your perspective, experiences, etc. when developing a website.

    Feel free to comment on:

    • suggestion to better solutions (or avoid using 'hacks' :D)
    • some nit-picks to a more beautiful and responsive front-end design
    • development cycle used for faster and efficient process of building a website.
    • other advice related to this challenge

    Challenges encountered:

    1. Border Radius. I used a hacky way to do it. Please see the live site > scss > media queries > border-radius.

    2. Sticky Footer. I used a hacky way to do this. (please see live site > html)

    • from html, I added class of h-100.
    • from body, class="d-flex flex-column h-100"
    • from main, I class="flex-shrink-0"
    • from footer, I used container class inside the tag

    Notes:

    • I do not have the same background as the one in the challenge. The repository that I downloaded does not have a background image included.
    Alex Kim 1,325

    @alex-kim-dev

    Posted

    Hey! A little suggestion on the border-radius. Do you know you can apply it only to the container along with overflow: hidden so it cuts the pointy angles of container's children? You can use it for larger screens, and for mobile just set the radius to 0.

    Also you can use Codepen's full page view link to make a nicer screenshot, however it's still going to mess up the a11y/html report.

    Nice job overall and have fun coding!

    Marked as helpful

    1
  • Alex Kim 1,325

    @alex-kim-dev

    Posted

    I'd suggest relying more on box model (margin, padding, borders, width) and normal flow than applying position: absolute (and thus creating more flows). Probably this is what's caused the issue. Good luck! 👋

    Marked as helpful

    1
  • P

    @emestabillo

    Submitted

    The side-by-side layout for medium devices looked awkward for me so I opted to stick with the stacked design. What's everyone's take on the tablet layout for this project?

    For devices with <667px height --> I couldn't get it to work unless I use absolute positioning. Does it make sense to still cover these devices? (Please be nice) Thanks for the feedback!

    Alex Kim 1,325

    @alex-kim-dev

    Posted

    I do basically the same as you did - leave the 1 column layout and give it max-width to prevent content (especially text) getting too wide + centering

    1
  • Alex Kim 1,325

    @alex-kim-dev

    Posted

    Yeah, the css variables are very helpful for this kind of purpose. The alternative set of color variables could be used with a selector that targets the checked state of the switch, eliminating the need to use JS for theme changing.

    2
  • Alex Kim 1,325

    @alex-kim-dev

    Posted

    Hi Connor, I've been inspecting some of your solutions and I think you're doing a great job! I like the visual accuracy and how the code is organized. There's one thing that I spotted - some BEM elements which are nested inside other elements have this type of naming: block__elem1__elem2__elem3. However it's not recommended in the BEM documentation (Guidelines for using elements - nesting). So instead of a markup like this:

    // pug
    nav.desk-nav
      ul.desk-nav__menu
        li.desk-nav__menu__item
          .desk-nav__menu__item__underline
    

    it's recommended to do it this way:

    // pug
    nav.desk-nav
      ul.desk-nav__menu
        li.desk-nav__item
          .desk-nav__underline
    

    Also I've noticed you have a lot of solutions using Svelte, so when I'll be trying it out I'll definitely come back to them and learn. Have a good one!

    2
  • Alex Kim 1,325

    @alex-kim-dev

    Posted

    Hey Mubaraq, great solution! Here are some of my thoughts:

    • I would use aria-label instead of adding a span with .visually-hidden on the buttons, your solution works perfectly though, so that's just a preference;
    • I would add a visually hidden meter element + wrap the paragraph above into a label for that meter, just to be sure that assistive technology will announce this part.

    Good luck and have fun making the next one!

    3
  • Alex Kim 1,325

    @alex-kim-dev

    Posted

    Hey Emmilie! That's a really fantastic work! Your solution is pretty accurate compared to the design. Also I like how you structured your SASS - very convenient and simple. I honestly don't know what can be improved, but here is a little hint on using media queries in sass. I noticed you often write @media screen and (min-width: 992px) {}. We can cut a little code here by defining mixins:

    // mdUp means for medium screens (992px) and higher
    @mixin mdUp {
      @media screen and (min-width: 992px) {
        @content;
      }
    }
    

    then we can use it like so:

    .class {
      padding: 1rem;
      @include mdUp {
        padding: 1.5rem;
      }
    }
    

    And that's it. Good luck and I'll see your next project!

    3
  • Alex Kim 1,325

    @alex-kim-dev

    Posted

    Hello, the reason for this is that the cards have fixed width of 350px. If you open the page on 375px wide screen the total width (card + margin & padding) will exceed that limit, and the page will have a horizontal scroll. Typically the mobile layout is made using width: 100%; to make sure it's always full width.

    0
  • Alex Kim 1,325

    @alex-kim-dev

    Posted

    Hi Izabella, I see you are using css grid for the layout, currently the grid has 3 columns and 3 rows, but both the 3rd column and 3rd row are not occupied by anything. For this kind of layout a 2x2 grid would be enough. Next up, the height of the boxes should be fluid - based on its inner content - so remove the height from each of them and use auto for grid rows. Css grid will make your bottom boxes of equal height automatically. That's it, nice job, keep up the good work!

    1
  • Alex Kim 1,325

    @alex-kim-dev

    Posted

    Great solution, Roshan! Pretty close to the design and responsive. There's a couple of points worth mentioning:

    • I don't think putting the whole component out of flow using absolute positioning is a good practice. The way to go would be to give the body 100vh height and position the component inside using flexbox.
    • the icon buttons aren't accessible to assistive technology users. Consider using <button> elements instead of divs + add styling for states (:active, :focus, :hover).

    Congratulations on solving your first challenge and good luck with the next one!

    1
  • Alex Kim 1,325

    @alex-kim-dev

    Posted

    Hey Adarsh, nice solution on this challenge! I like that you added some animation for cards, cool little effects on hover. And that fancy font that you used for the attribution is looking great. I looked into the code and i have some feedback for you:

    • the borders on top of the cards are slightly curved which is not in the line with the design
    • for 555-768px screen sizes the cards are shifted to the right; also i think for mobile screens (375, 425px) the layout should be 1-column
    • the height set for every card can cause issues when the content inside changes
    • the card icons are not pinned to the bottom of the card, if i change text ab...
    • there are some css grid properties that can help with aligning the left & right cards vertically
    • i think having 7 screen breakpoints is a little too much

    Good luck!

    1
  • Alex Kim 1,325

    @alex-kim-dev

    Posted

    Hey, your solution looks really well! I've just inspected it and there's some points worth mentioning:

    • the component gets squashed on tablet size screens, giving it max-width and auto margins on the sides could help with that
    • there's an error in the console which comes from @import statement in the css file, wrapping the url address into url() function should fix it @import MDN. Also it's a bit more benefitial in terms of performance to load fonts using <link> Stackoverflow question
    • the sass code for the button states styling doesn't work in browsers, you have to compile it down to plain css, or just rewrite it as css

    Keep up the good work!

    1
  • Alex Kim 1,325

    @alex-kim-dev

    Posted

    Nice try on this challenge! Here are a few things you can improve:

    • i think the background for this challenge is intended to be fullscreen
    • not all text elements use provided fonts
    • try to use custom js validation instead of the native one, add feedback messages for empty inputs as mentioned in the design
    • interactive controls' states should be styled (:focus, :hover, :active)
    • using floats for layout building can cause some problems, more appropriate solution is to use flexbox / grid
    • i don't think the upper blue thing should be a button, to me it's a heading inside of the form
    • try to be more descriptive with the values of aria-label, eg 'Signup form' instead of just 'form'
    • there's no need to put br between the inputs
    1
  • Alex Kim 1,325

    @alex-kim-dev

    Posted

    Hey Kushank, nice work on this challenge! I have some feedback for you:

    • cards have different heights
    • h2 is skipped, more info: heading elements MDN at "Accessibility concerns" section
    • repeated styles from the cards could be extracted into a separate class, allowing code reuse
    • responsiveness:
      • content is getting stretched too much on big screens, max-width can help with that
      • the layout of pages for 500-1100px screens can be improved
    1
  • Alex Kim 1,325

    @alex-kim-dev

    Posted

    Wow. That's a great pixel perfect solution, Simon! And there's a lot of clever sass techniques. I will definitely look more closer into your code to learn something new.

    2
  • Alex Kim 1,325

    @alex-kim-dev

    Posted

    I haven't tried SMACSS myself but your css looks very clean. As always, nice and pixel perfect solution!

    0
  • Alex Kim 1,325

    @alex-kim-dev

    Posted

    I have two questions.

    1. Do you really need to add aria-hidden attribute to the div with a background image? I think screen readers won't annouce it anyway.
    2. Why are you turning autocomplete off for the form?

    I like your solutions, learned some good practices while inspecting them. Thanks!

    0
  • Ksenia 1,350

    @ksenius

    Submitted

    I tried hard on this challenge. It took me hours to configure Gulp for the first time. I also used CSS Grid for the first time (only in the footer, though) and made it work in IE 11. But the most challenging thing to me was to position background images.

    Feedback is welcome and appreciated :)

    Alex Kim 1,325

    @alex-kim-dev

    Posted

    Great work! The design, how the css is organized - it's just awesome!

    I noticed i can't swipe the testimonials card again if the previous animation is not ended.

    Looking forward to your next solutions.

    0
  • Alex Kim 1,325

    @alex-kim-dev

    Posted

    Pretty decent job! I like your gulp setup and the way you used BEM. Also the page matches the design very well! There're just 2 small things that i would add:

    1. styling for controls :focus state
    2. rel="noreferrer" attribute for the external links, here is more info
    0