fac-11/week2-atoms

week2-atoms's Issues

create a function show and its test

create a function that shows the tasks

Could clarify test descriptions

According to the Intro to TDD, tests should have this format:

// Answer these questions for each unit test you write:
test('What component aspect are you testing?', function(assert) {
  const actual = 'What is the actual output?';
  const expected = 'What is the expected output?';

  assert.equal(actual, expected, 'What should the feature do?');
  assert.end();
});

Currently lots of them have the same strings in both descriptions, and they're not always very clear what's being tested e.g. 'Should return an array'
https://github.com/FAC-11/week2-atoms/blob/master/test.js#L229

Mobile responsiveness needs attention

When the page shrinks the contents start to shrink as well after around 950px. We need to adjust this with a media query for our disorganised big fingered friend as it is one of the user stories!

I've done this - Maybe have that appear only once something has been added?

It doesn't seem to be immediately clear as soon as we go on the site what the I've done this is for. Maybe have it appear only after somethings been added to the todo list?

create render function and its test

when introducing the task in my "input form", display in the DOM. And also load up store tasks

Buttons have no focus styles

You have outline: none set for all your buttons. If you're going to remove the browser defaults it's really important to replace them with something so that keyboard-only users can see what they're interacting with.

toggle function

complete the marktodo function to do the toggle;

Declare variables at the top of their scope

It's good practice to declare variables at the top of their scope, e.g. at the top of the function which contains them:
https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Statements/var (see var hoisting section).

Inputs should have labels !

Accessibility audit [Severe] Controls and media elements should have labels (2)
refers to : https://github.com/FAC-11/week2-atoms/blob/master/index.html#L20-L23

Just add some labels!

some guidelines are here: https://github.com/GoogleChrome/accessibility-developer-tools/wiki/Audit-Rules#ax_text_01

Make sure you're running tests with the correct function

This is listed under your 'add to do' tests, but seems to call deleteToDo() https://github.com/FAC-11/week2-atoms/blob/master/test.js#L120

For readability, you could add a code comment for markToDo tests at https://github.com/FAC-11/week2-atoms/blob/master/test.js#L171

sortTodos should have a new name

This should have a new name cos it is not sorting the elements, it is divided the todos in completed and not completed

files in repo which shouldn't be there

the .idea folder, the .bash_profile and .DS_Store shouldn't be in the repo, please remove them and in future make sure not to push them up to github in future.
If you add them to the .gitignore at the start of any project you should be all good. Not all that sure how they got in there, but if something one of you is using is creating them plz make sure they go in the .gitignore.

create function add and its test

create a function that adds the task I write in the the "input form"

--> todos argument should be unchanged
--> new extended array (it should contain todos with the newTodo)
--> add an id to new todo -use generateId function-

Delete merged branches

its good to keep branch numbers to a bare minimum to keep everything nice n tidy :)

Well done !!

Awesome job !!

• It looks very clean ! I like the long white bar that surrounds a todo list !

• The site works ! Its clear you have all done a very good job !

create a function "markAsComplete"

create a function that will mark my tasks as complete in my todoapp

• The todos argument-array should be unchanged globally
• IdtoMark should be added;
  -All other elements should be unchanged, except one with IdToMark
  -Toggle Value

Save functionality not working online

Save functionality not working online. When refreshed the items don't stay. Fix for online use.

100% Coverage!!!

good work pals! 💯

Object.assign is ES6

I know it may have confusing as you may have been told about it as a way to copy an object, but Object.assign is ES6 and as such doesn't have enough browser compatibility that we at FAC consider it to be okay to use in client side javascript.

If your ever are unsure of whether something is ES5 go to the MDN page of the function/method/thing you want to use and scroll down to the "Specifications" section.

adding a new todo doesn't work

When I write some text into the input and click the "+" button nothing happens. I would expect a new todo to be added and for it to be appended to the DOM.

The site looks great in terms of the design and CSS but plz make sure the basic functionality is there before shifting to making everything beautiful.

Remove commented code

I know its really tempting to leave commented code in a project if you think you're going to use it again later down the line.

In reality the scope of projects change quite a lot so you can end up not needing features you previously did think you would need. In this case that might not be true but its a good habit to get into. Also the whole benefit of git is that you can go back and look at the history so if you delete it instead of comment it you'll always be able to find it again!

https://github.com/FAC-11/week2-atoms/blob/master/test.js#L101-L107

Avoid deeply nesting CSS selectors

Some of your CSS selectors are really deeply nested (ul .completed li .buttons button.complete svg. This unnecessarily adds specificity to your selectors, which means they're much harder to override. It's also a lot harder to read exactly what this CSS is styling.

Generally it's better practice to just add a class to the element you want to style (in this case the SVG). This is the philosophy behind practices like BEM.

There's a lot more info here: https://cssguidelin.es/#specificity

Form is better for creating todo

Right now you are not using a form correctly to create a todo

Something more like

<form>
  <input type=text>
  <input type='submit'>
....

Then in js

form.addEventListener('submit', function(event) {
  //now event object has all the data we need attached, the the click handler does not need to query the dom ..
}

Generally more accessible to mark up the form

deleteToDo() doesn't clone objects

Not sure if this is an actual issue but:

The deleteToDo function filters the original array so it does return a new array with the deleted item missing, however I think each item of the new array will be a reference to the objects in the original array, rather than copies of the objects.

So although the delete function itself doesn't edit the original it doesn't return a completely new array - not entirely sure if this is an actual issue, as if you then went on to edit the item presumably the edit function would clone each object before making the edit...

https://github.com/FAC-11/week2-atoms/blob/master/logic.js#L35

edit the to-dos items

I wil need a function that will allow me to edit my tasks in case I want to amend them

cannot tab !

I cant seem to tab through the site, especially to the delete todo button or add to do button

Use of Github

As far as I can see none of your commits are linked to issues. This is really important!

It's a big part of good git/github workflow.
Among other things, it allows you to see exactly which commits link to which issues, so if an issue is closed, but you see that it's actually not closed, you can see which commits did what relating to that issue.

Please make sure in next weeks project you link every commit to an issue.

Sizing doesn't scale on mobile

All of the text stays dead small on mobile (this is on iPhone 5):

If you add a viewport meta tag that should help.

Coverage report needed for pre review build

We should run a coverage report to test out for any unused or rogue code at this stage.

tasks should be big enough

There should be enough space so that I dont hit the wrong task with my thumb.

--> css style.

add language tag !

html lang="fr"

eg ^^

perhaps change title of your tests?

Instead of having to repeat yourselves when naming all of your tests, it might be easier to take a different approach. For example, for your first test, instead of repeating the phrase: 'should return an array' on the following lines:

https://github.com/FAC-11/week2-atoms/blob/master/test.js#L5 (first line)
https://github.com/FAC-11/week2-atoms/blob/master/test.js#L8 (second line)

It might be more clear to always keep the first line to be the name of the function this test refers to . i.e. "deleteTodo function'.

Then have the second line to be the actual feature this test will test, so keep it as "should return an array".

Usage of deepEqual

on this line: https://github.com/FAC-11/week2-atoms/blob/master/test.js#L8

since you're not testing a nested structure, and just that the result is an array you can probably just use t.equal()

make a 'completed item list section'

In order to see the completed tasks, create a function that will display the completed tasks in another section

Edit function

Code exists for edit function but feature not added as modal fields need work before hand. A later focus of the project but just a note code exists

visual display of tasks

when clicking on the task, it will get bigger/highlighted to make more visual the task

Should update local storage in update function not render function..

The render function is responsible for creating the todo list and updating the dom.

localStorage.set would be better in the update function.