Hi everybody! I am new here, so please comment, all your suggestions are very helpful to me! As for the task, probably it could be done in a more simple and straightforward way, but I decided to make it with Redux/ Typescript. The most difficult part for me was the drag-and-drop section, because I used it for the first time here. All the code is written by myself, except for that drag-and-drop part, which I found somewhere else, but I learned how it works for now. I know that there is plenty of space for improvements, so please do not hesitate to give me an advice! Thanks a lot!
Pablo da Silva Bezerra
@PabloBezerraAll comments
- @StasMelamedSubmitted 11 months ago@PabloBezerraPosted 11 months ago
Hello! How are you! Great work! I still don't really understand how react works, so I can't give my opinion on the code itself. However, there are some features that I have to highlight.
- In the challenge the drag and drop functionality was used to sort the tasks and not to insert them into the task field. I thought it was a great idea, but it's not very intuitive. You can keep this feature, but you need to come up with another way of adding tasks.
- The drag and drop functionality in the task field doesn't work very well, some tasks duplicate instead of moving the task. Possibly this is because of the add tasks when moving functionality you've implemented.
- And finally, I can't delete a specific task by clicking on the X, instead of deleting it, the task is marked as completed. You can solve this by adding the functionality to mark the task only on clicking on the circle and not on the whole task.
These are some performance irregularities I've found. I hope I've helped you improve your project! See you!
0 - @MachadoASubmitted 12 months ago@PabloBezerraPosted 12 months ago
Hello! Great work! Really a project true to the challenge! However, there are a few topics that caught my attention:
-
Your Html is a bit confusing, especially in the email input. You placed a
<div>
without class that inside it there is an<article>
that there is a<label>
that references an input that is outside the<article>
a solution to this would be the<form>
that would look more or less like this:<form> <label for="email">Email address</label> <p class="alert">Valid email required</p> <input type="email" name="email" id="email" placeholder="[email protected]"> <input type="submit" value="Subscribe to monthly newsletter"> </form>
Note: this way the addEventListener in your script should change from
'click'
to'submit'
, so that the event is also triggered by pressing the enter key on the keyboard in addition to clicking the button.- In addition, the
<article>
tag does not make sense in this position. I recommend taking a look at this site to better understand what I'm talking about - I miss a
<main>
tag in your project, this tag unifies the main content of your site, it would work well if you replaced the<div class="conatain">
with the<main>
tag, this will make it more semantic. - Likewise
<div class="attribution">
which would be much better represented by the<footer>
tag. - Finally, a bit about style. When the width of the screen drops below 900px, the background of the page becomes all white (I don't know if that was the main idea, but it happens). The
:hover
of the button is a little off, adding a transition will make it more visually elegant, you can do this by adding thetransition: all ease-in-out 1s
. And finally there's vertical scrolling, which in my view is a bit annoying, you can fix this by adding a height of 100vh in the body, this will make the entire body of your site occupy 100% of the viewport.
And that's it, I hope I've helped you improve your project, which, despite everything, looks magnificent!
Cheers!
Marked as helpful0 -
- @eldmarSubmitted 12 months ago
Feedback is very welcome!
@PabloBezerraPosted 12 months agoCongratulations for your work! It was very faithful! However, what caught our attention the most was that the title of the card has an
<h3>
tag, ideally it should always start with an<h1>
tag, if it doesn't have the desired size, just modify it in the CSS . Another thing was that you imported a text font directly into the HTML, the ideal is to import style fonts into the CSS document itself with @import. Hope this helps! Hugs!Marked as helpful0 - @daxvinciSubmitted 12 months ago
while building the project i couldnt get the h2 and paragraph text to align, what should i have done differently?
@PabloBezerraPosted 12 months agoComo vai cara? Tudo em cima? Respondendo a sua pergunta, primeiro que não existe um h2 no seu código existe apenas 2 parágrafos de tag <p>, segundo que para alinhar ao centro da tela (que eu suponho que foi nesse sentido a sua pergunta) você poderia ter utilizado o "text-align: center;" isso fará com que fique centralizado. Mas reparei que vc publicou o readme e não o site em si, pra resolver isso você deve remover o documento "index.html" da pasta e deixa-lo fora de qualquer outra pasta, ai o github pages vai identificar o site e vai publica-lo corretamente. Espero ter ajudado! Abraços!
1