facn2/apolloxii

Autocomplete Widget

apolloxii's Issues

No tests at all :(

Maybe start with writing a test for the searchFunc function?

Requiring unnecessary modules in `server.js`

We're not using these in server.js, so we don't need to require them in there 😊

const fs = require('fs');
const path = require('path');

ES6 on the frontend in `script.js`

Remember that for now we're still writing only ES5 on the frontend as it's not fully supported. I think it's only a case of changing const to var on line 26 and change the arrow function. 👍

Improve sintax

Improve Syntax for router and search files

Uneven contribution/commits

Hey guys, I think it's really important to always check out your contributions and see if you're spreading the workload evenly amongst your team. I know that @KatiaAsh was having laptop issues, and so if she was working on Stefano or Vered's devices and is missing commits from her own GitHub account that's completely fine, but just something to remember that what makes us so special is that we always encourage pair programming!

Unnecessary console.logs and commented out variables

For instance line 12 of router.js

Router/handler

Again, nice start to modularisation and seperation of concerns, but could be improved.

All the router should do is pass the (req, res) onto a handler, and then the handler does everything with it. So you have

router = (req, res) => {
if (option1) 
    handler1(req,res)
if (option2) 
    handler2(req,res)
}

Everything you need is within those request and response arguments. This then is much neater, easier to read and easier to replicate.

If for example, one person writes the router, they can always pass the (req,res) arguments to every handler, and then whoever is making the handlers will always know to make a function with those two arguments

Search function non pure?

Really nice modularisation of all your code, but you're search function relies on the food variable to work, so testing it you would also have to require in food - maybe pass it as a second variable to the function (JSONToSearchThrough?) and then you've got a nice neat, easily testable function.

Mixing ES5 and ES6 Syntax in `search.js`

What's the reason behind mixing ES5 and ES6 syntax?

Some good reads:
https://medium.com/javascript-scene/javascript-es6-var-let-or-const-ba58b8dcde75
https://javascriptweblog.wordpress.com/2010/07/06/function-declarations-vs-function-expressions/

Loading FontAwesome vs SVG in `index.html`

Just for the future, it's probably better to use SVG vs importing in the entire FontAwesome for better performance, on line 10:

<link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/font-awesome/4.7.0/css/font-awesome.min.css">

Autocomplete not accessible

Keydown enter of a search takes me to 404
Cannot tab through search results
Cannot select food using key up or key down

Search function

Create the basis for the search function

Unclear commit messages

Try to steer clear from vague commit messages like:

"fix somestuff in css"
"Made some small changes to router"
"changed css and handler"
"edit html and add images"

You get the idea 😊

Unused CSS rules

see Audits

landing page for results sreach

Remove console.log

A few unnecessary console.log (that's been commented out). Consider removing if it serves no purposeful function

create files

creating files for public, src, assets ...

I think the space is messing up your autocomplete feature

If I'm typing in anything followed by a space, the autocomplete words go away. It works if it has a ,

Using both `===` and `!==` in `router.js`

Okay, I know they're the same, but it's probably best practice to stick to one or the other:

Not responsive

I can't use the app on mobile because it's not designed to be responsive 😢

update html

Link for webpage

Add link in README to webpage

Basic css and change html

• basic design
• change file names in html

the variable result.obj is redundant

Line 6 of Search.js

results still populated after clearing input

Clearing input should result in clear search field - instead, I get a list of the first objects of your food directory

Basic HTML

Add structure and main elements on index.html

why exporting server.js function?

Any reason why?

README.md file

Write the basic content for the Readme file

specify image dimensions

see Audits

Unused variable in `search.js`

What is var resultObj = {}; meant to do? On line 6 😊

Create server

Create and build server.js, handlers.js, router.js

Unused/unstyled HTML tags

What is this doing on line 24?

<i class="fa fa-send"></i></button>

✨ ✨ ✨ Awesome job! ✨ ✨ ✨

Hey team, after code-reviewing I feel like your team understood the objective of this week and completed a project that outlined your understanding, really awesome job! As always, when we learn something new we tend to drop the things that we learned from weeks before (I completely understand, I've been there!) But it's good to always take the time to go back and spend some time on the stuff you might've not prioritized (for example, accessibility and tests). Cheers! 👍

dom and search function

path and fs not necessary

No need for path and fs in your server.js if it serves no function in there

Add image folder

Change logo from Apollo11 to REAL Apollo

fix css