Skip to content
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

NW6 | Orlando_Morales | JS2_Module_Todo-List | Week3 #213

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

OrlandoMoralesKuan
Copy link

Learners, PR Template

Self checklist

  • I have committed my files one by one, on purpose, and for a reason
  • I have titled my PR with COHORT_NAME | FIRST_NAME LAST_NAME | REPO_NAME | WEEK
  • I have tested my changes
  • My changes follow the style guide
  • My changes meet the requirements of this task

Changelist

Briefly explain your PR.

Questions

Ask any questions you have for your reviewer.

Copy link

netlify bot commented Jan 22, 2024

Deploy Preview for cute-gaufre-e4b4e5 ready!

Name Link
🔨 Latest commit 8a97169
🔍 Latest deploy log https://app.netlify.com/sites/cute-gaufre-e4b4e5/deploys/65b00c0668b535000823343c
😎 Deploy Preview https://deploy-preview-213--cute-gaufre-e4b4e5.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

@bunday bunday left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a very good design, really like the look and feel. Easily one of the best I've seen so far.

However, some crucial requirements are missing:

  • Setting deadline for todos
  • Ability to mark a todo as completed
  • Clearing Completed todos

<div class="wrapper">
<header>Todo-List</header>
<div class="inputField">
<input type="text" placeholder="Add your new Todo" />
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

user's should enter the todo, and also set the deadline date, the date input field is missing

</ul>
<div class="footer">
<span>You have <span class="pendingTasks"></span> pending tasks</span>
<button>Clear All</button>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't need a clear all button from the requirements, we need a "Clear Completed Todo" button/feature

const todoList = document.querySelector(".todoList");
const deleteAllBtn = document.querySelector(".footer button");

inputBox.onkeyup = () => {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is good


// These are the same todos that currently display in the HTML
// You will want to remove the ones in the current HTML after you have created them using JavaScript
let todos = [
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

by default, these tasks should be displayed on the home page

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants