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

Submitted

Todo app using ReactJs and Sortable JS

#react#sass/scss
Cường 845

@docuong0912

Desktop design screenshot for the Todo app coding challenge

This is a solution for...

  • HTML
  • CSS
  • JS
3intermediate
View challenge

Design comparison


SolutionDesign

Solution retrospective


I haven't figure out how to deal with number of tasks if it exceeds the screen, any idea with that?

Tell me if there's any problem with the solution or give me some advice in coding.

Thank you for your time!

Community feedback

Miroslav 180

@miroslavdurin

Posted

Hello and congratulations on completing this solution.

I would suggest that you add a class .scroll if a list is longer than 9, or whatever number you like. You can change container div inside of ToDoList.js to something like this:

<div className={`container ${theme?"dark-theme":"light-theme"} ${list.length >= 9 && "scroll"}`}

Inside Main.scss you can change to this:

.container {
        @media (max-width:$mobile-width){
            margin-top: 20px;
        }
        margin-top: 50px;
        max-height: 500px; /* You can put any height you like */

        /* This class is added if list.length is bigger than 9 */
        &.scroll {
            overflow-y: scroll;
        }

        /* Custom Scrollbar styles */
        &::-webkit-scrollbar {
            width: 10px;
            
            cursor: pointer;
        }
          
        /* Track */
        &::-webkit-scrollbar-track {
            box-shadow: inset 0 0 5px grey; 
            border-radius: 10px;
        }
           
          /* Handle */
        &::-webkit-scrollbar-thumb {
            background: var(--light-grayishblue-hover); 
            border-radius: 10px;
            
        }
          
          /* Handle on hover */
        &::-webkit-scrollbar-thumb:hover {
            background: var(--dark-grayishblue); 
        }
    }

I've added some styles for a scroll-bar too.

There is also issue with a check mark, which is not centered. I think that inside of .todo-box img you can remove these 2 lines:

right:2px;
top:5px;

And I've noticed that on a screen bigger than 1450px background-image is not full width. You can just add background-size: contain inside of .App class.

Marked as helpful

2

Cường 845

@docuong0912

Posted

@miroslavdurin that's a great idea, really detail explaination, i will give it a try, thank you for your time and effort

1
Jon Wright 160

@Dr-Wrong-Mo

Posted

I see the problem with your background exceeding your content. There are a few items you need to change.

The primary issue is that your div with the class "todo-box" has a position of absolute (I'll start referring to this as div.todo-box or just .todo-box). The property absolute position takes it out of the normal content flow. It is now positioned within the closest element that has position relative. If no ancestors have position relative, it becomes relative to the window.

I suggest you study CSS Flex-Box and CSS Grid for your layout properties.

The reason this is important is that your primary element (div.App) is now functionally empty.

Position absolute should be used sparingly, and almost never on your primary content.

Here is what I changed to make it work:

  1. On div.todo-box, remove "position: absolute"

  2. On div.App, change "height = 100vh" to "min-height = 100vh"

This will fix the problem where your background cuts off, but it will mess up the formatting for the child elements within div.todo-box. It will also make your todo-box align to the left side of the screen.

You can recenter your content by modyfing div.App as follows:

display: flex; justify-content: center; padding-top: 100px;

This will look better but your dark mode toggle will still have absolute positioning, which will move it to the top-right of the screen.

Remove position absolute (as well as the top and right properties). This will place it directly under your heading.

I would wrap the heading and the image in a parent div. Let's call it div.heading. So it would look like this:

<div class="heading"> <h1>todo</h1> <img .../> </div>

Now the CSS

.heading { display: flex; justify-content: space-between; }

This has almost worked, but your hidden icon is blocking the shown icon from being where it wants. This is because you used "opacity: 0" to hide it. Opacity does not remove the item from the flow of the body. It only makes it less visible, but it is still taking up space. What you can do instead is apply "display: none". This will remove it from the flow of the body, allowing your shown icon to fit where you want.

Your icon will need to have some new properties applied to get it to have the shape you want.

A few other notes:

I didn't like the interface of needing to click on the checkbox in order to submit a new item. It would be better if I could press enter to submit the item, clear the input field, and place the cursor back into the now empty input field, ready for another new item.

Also, the positioning of the checkmark within the checkboxes is off. In my previous comment, I mentioned that you should study up on Flex-Box. This is another expample of where you could use Flex-Box (or Grid).

The project looks decent otherwise. Good job!

Marked as helpful

0

Cường 845

@docuong0912

Posted

@Dr-Wrong-Mo omg, i violated a lot basic mistake, thanks for pointing that out for me, i did applied flex box in the checkbox but some how the check image didn't fit in.

About the opacity, i want to make an animation when toggle the theme change button. After setting its opacity to 0 i also set the z-index to -1, which i personally think won't affect much to other elements.

Lastly, the interface, as AngeliqueDF mentioned, i will fix it later.

Thank you a lot for your time and effort. i really appreciate it.

0
Jon Wright 160

@Dr-Wrong-Mo

Posted

@docuong0912 I see what you are saying about using opacity for animations.

I see the problem with that. You can't change both display and opacity at the same time.

Here is a simple solution to overcome that. You give the element two separate classes. One for display none and one for opacity 0. You toggle the display class first, set a timeout for 1ms or more, then toggle your opacity class.

Here is an example that shows it in action. Notice that the button moves when it is clicked, because the heading is removed from the document flow.

https://codepen.io/Dr_Wrong/pen/WNzvmKw

Marked as helpful

0
Cường 845

@docuong0912

Posted

@Dr-Wrong-Mo thank you sir, how thoughtful of you, new knowledge acquired, i'll fix it later. Are there any problem left to solve,sir?

0
Jon Wright 160

@Dr-Wrong-Mo

Posted

@docuong0912 It looks like you haven't handled the mobile design, so you could work on your media queries too. Keep it up!

I'd love to see it again when you have polished it up more. If you don't mind, reply to one of my comments when you're done, so I can check out your finished product.

0

@AngeliqueDF

Posted

Hi Nguyễn, this solution looks great!

About your question, I think the best way to adapt the layout when there are a lot of tasks is to use CSS Grids for the layout.

I would apply the background on the body element, give it a min-height: 100vh;. Then use display: grids; to create the layout. I haven't tried this yet but I think it could work.

We have similar solutions and I didn't think of this case, so thanks!

Small feedback: can you make it so that we can add tasks by pressing the Enter key? It might be more intuitive than pressing the button.

Marked as helpful

0

Cường 845

@docuong0912

Posted

@AngeliqueDF i don't really understand your solution, but the feedback is helpful, thanks for your idea

1

Please log in to post a comment

Log in with GitHub
Discord logo

Join our Discord community

Join thousands of Frontend Mentor community members taking the challenges, sharing resources, helping each other, and chatting about all things front-end!

Join our Discord