-
Notifications
You must be signed in to change notification settings - Fork 425
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Project Todo #442
base: master
Are you sure you want to change the base?
Project Todo #442
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love the concept, design and minimalism of this app! It is very user friendly and feels inviting. Feels analogue in a good way. The code is precise and effective, follows all the requirements for this project. It's responsive. I have no major comment about what could be improved, everything works well (but I wrote some small suggestions in my comments).
P.s. please provide a live link! d.s.
@@ -15,7 +15,7 @@ | |||
"parser": "@babel/eslint-parser", | |||
"requireConfigFile": false, | |||
"sourceType": "module", | |||
"ecmaVersion": 2017, | |||
"ecmaVersion": 2020, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good thing to do. Was it helpful?
"eslint": "^8.21.0", | ||
"eslint-config-airbnb": "^19.0.4", | ||
"eslint-plugin-import": "^2.26.0", | ||
"eslint-plugin-jsx-a11y": "^6.6.1", | ||
"eslint-plugin-react": "^7.30.1", | ||
"eslint-plugin-react-hooks": "^4.6.0", | ||
"moment": "^2.29.4", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Never used moment, interesting!
deleteSingleTask: (store, action) => { | ||
const id = action.payload; | ||
const copyofTaskArrayFromStore = store.items; | ||
const condition = (element) => element.id === id; | ||
const foundIndex = copyofTaskArrayFromStore.findIndex(condition); | ||
copyofTaskArrayFromStore.splice(foundIndex, 1); | ||
store.items = copyofTaskArrayFromStore; | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting way of doing it, but I believe there could be a simpler way of doing the same thing, maybe?
store.items = copyofTaskArrayFromStore; | ||
} | ||
} | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, the reducer is easy to follow and tidy. Good job!
import { task } from 'reducers/task'; | ||
|
||
export const AddTask = () => { | ||
const [inputValue, setInputValue] = useState('Add items here...') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting to put the placeholder text as an initial state, but I noticed it disappeared after the first use and then it was more difficult to find the text input field. Maybe a better solution would be to use an actual placeholder text in the <input>
tag?
.paper { | ||
width: 98vw; | ||
height: auto; | ||
min-height: 80vh; | ||
background-color: #fff; | ||
box-shadow: 3px 2px 2px 0px rgba(0,0,0,0.4); | ||
position: relative; | ||
padding: 40px 0 40px 0; | ||
margin: 20px 0 10px 0; | ||
} | ||
.paper::before { | ||
content: ''; | ||
width: 2px; | ||
height: 100%; | ||
background-color: rgba(255, 0, 0, 0.6); | ||
position: absolute; | ||
top: 0; | ||
left: 40px; | ||
} | ||
.pattern { | ||
min-height: 80vh; | ||
height: auto; | ||
background-image: repeating-linear-gradient(white 0px, white 24px, teal 25px); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is so impressive you created the note paper in css! It's inspiring! Great work!
.container { | ||
display: block; | ||
position: absolute; | ||
margin: 2px 0 0 10px; | ||
cursor: pointer; | ||
font-size: 22px; | ||
-webkit-user-select: none; | ||
-moz-user-select: none; | ||
-ms-user-select: none; | ||
user-select: none; | ||
right: 15vw; | ||
} | ||
.container input { | ||
position: absolute; | ||
opacity: 0; | ||
cursor: pointer; | ||
height: 0; | ||
width: 0; | ||
} | ||
.checkmark { | ||
position: absolute; | ||
top: 0; | ||
left: 0; | ||
height: 0.7rem; | ||
width: 0.7rem; | ||
border: 1px solid rgba(0,0,0,0.2); | ||
border-radius: 2px; | ||
box-shadow: 1px 1px 2px rgba(0,0,0,0.2); | ||
} | ||
.checkmark:after { | ||
content: "✓"; | ||
position: absolute; | ||
display: none; | ||
font-size: 1.4rem; | ||
color: rgb(0, 170, 0); | ||
font-family: monospace; | ||
font-style: normal; | ||
line-height: 0; | ||
margin-top: 4px; | ||
margin-left: 1px; | ||
} | ||
.container input:checked ~ .checkmark:after { | ||
display: block; | ||
} | ||
.container .checkmark:after { | ||
-webkit-transform: rotate(348deg); | ||
-ms-transform: rotate(348deg); | ||
transform: rotate(5deg); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work making the checkboxes! Minimalist and stylish!
input { | ||
width: 150px; | ||
border: none; | ||
background-color: transparent; | ||
margin-right: 10px; | ||
font-family: 'Shadows Into Light', cursive; | ||
outline: rgba(0,0,0,0.2); | ||
font-size: 1rem; | ||
line-height: 0; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really like how the input field is hidden, even though it's hard to find once used.
@media (min-width: 426px) { | ||
body { | ||
height: 100%; | ||
background-image: url(../public/bree-anne-XnpWqPZNfXE-unsplash.jpg); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this might be a repetition/ background-image is used twice.
} | ||
@media (min-width: 426px) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job on responsiveness!
No description provided.