facn3 / mashrou_ghazal Goto Github PK

In handler.js you are using a mixture of ES5 and ES6. Eg

ES6 arrow function

fs.readFile(__dirname + '/words.txt', (err, data) => {
  if (err) {

ES5 function

function handler(request, response) {
  var url = request.url;

For consistency I would suggest picking one and sticking with it. In the back end use ES6 where possible 👍

there is an unused div in your html!

🐐 🐓 🐕🐖 🐁 🐂🐲 🐡 🐊
🐪 🐆 🐈🐩 🐾 💐
🌸 🌷 🍀🌹 🌻 🌺
🍁 🍃 🍂🌿 🍄 🌵🌴 🌲 🌳

Deleted unnecessary comments and console.logs

Not sure how this has got here but in the text file there is this from a merge at some point.

<<<<<<< HEAD

Make sure when you merge with master all conflicts are resolved and you have removed all of the things above.

Its on line 1 and also at the end of the file.

No tests yet? The file and packages (tape and tap-spec) shouldn't be there if they aren't being used at all. I understand its hard to test this week but some tests on the ways you are filtering data would have been nice.

The autocomplete function is actually an xhr request and should probably be renamed to something that suggests this.

I would also add it might be nice to make the xhr function a generic and reusable function. This could then be reused to send an xhr request to somewhere else and actually render some search results on the page.

I am typing and getting suggestions but are the suggestions all the words ever?! Or have you got a theme for your website? If you do that should be made clear to the user 🤷‍♂️

It is good practice to put images for the readme in an issue then use the url in the issue. This means if you have lots of images in the readme they aren't in the way of the images actually for your project. It's not a big point but just worth noting for the future.

Creating the file structure for the project

i.e.

• lib thats for the handler.js files
• src server-side .js files
• public an html files, js folder and css folder
• test the test files

At the moment when you submit the search it takes you to a 404! What is supposed to happen at this point?

this line will run on every call of handler.js, this is not the way to go, your dictionary file needs to be read once and only once when you start your server, you can do this by creating a function and exporting it, that function should be called from your router, check #47 first

• Create an auto-complete function in the back-end.

the server sends back an error about not finding the icon if you don't wanna add one do not send back an error about not finding one and unlink it.

small issue 😸

Avoid the use of any global variables, for example, your arr variable in handler.js, there shouldn't be a need for the use of global variables.
try to extrapolate the functions that actually use that variable and then write a wrapper to them where the variable is only defined there

Linking index.js to handler.js

this file is not a backend source code and therefore should not be in the src folder if you still want to keep in src then adding a folder called dictionary helps.

The search bar looks huge, however if you type a really long word the text starts disappearing as another element hides it.

It looks from the url you have supplied in the repo that you are hosting on heroku

http://immense-ravine-82923.herokuapp.com/

In your package.json you have installed @google-cloud/nodejs-repo-tools which is for a google cloud service. Is this being used still seeing as you have heroku? If it is unused it should be uninstalled and the app.yaml file removed as well.

Customizing the index.html

I.E

• meta data
• form
• title
• script and style refernces

the website is not a very responsive one, also you should work on the mobile version a bit to make it easier to be used.

😄

Overall, your github flow is nice 🎉

issues
I love the titles and descriptions for your issues, they show what you are working on, which is great! Don't forget to open an issue for every feature/part of code you are working on.

commits
You almost always reference an issue when you commit changes, which is great! ✨
Don't forget that if you don't have an issue, you can open one before you commit your changes, but of course the best option is to create an issue before starting to work on something.

pull requests
Your pull requests have better titles and descriptions, and most of them reference an issue. Try to assigns your team members as reviewers, so all of you go over the code before merging it. This is your chance to approve ✅ request changes ✖️ or ask any question about the code ❓
Please don't make any changes on master 😱 even if it seems like a small change. Every change should come from your code editor and terminal.

contributions
Try to aim to split the work evenly between all of the team members. You can see the current graph here.
You can do it by making sure you swap roles and laptops all the time ⏲️

Keep up the good work! 😄

picking it up in the audit - perhaps a typo.

it would be cool if there was a way to know if there are no results found for the autocomplete

☀️☀️☀️☀️☀️☀️☀️☀️☀️☀️☀️☀️☀️☀️☀️☀️☀️☀️☀️☀️☀️☀️☀️☀️☀️☀️☀️☀️☀️☀️☀️☀️☀️☀️☀️☀️☀️☀️☀️☀️☀️

There is commented out code here which should be removed.

You shouldn't be using px in your css anywhere. Use %, em or rem for better responsiveness.

Creating a drop down menu with pre-setup words from the auto complete to choose from

Would have been good to see more on the following:

• User instructions - autocomplete I get, but what is the dataset that i'm searching through?
• I've read some of the what but none of the why
• How did you complete the work? build process

there is a bit of a functionality problem < because that your data is just one word its passing but because your always searching for the the last word , if I searched for one term of two words that you have it in the data as one term of two words it wouldn't find it because it is going to look for a term that starts with the second word in your search term.

good idea to use the url.parse to parse it into a url object.
good job guys 👍 😃

i'm not sure if its possible because our group didn't implement datalists, but would it be possible to hide the arrow icon? not necessary as the dropdown appears automatically....

😊😊😊😊😊😊😊😊😊😊
😊😊😊😊😊😊😊😊😊😊😊😊😊
😊😊😊😊😊😊😊😊😊😊

💇 💅 👦 👧 👩

Love the fact you are including a diagram of the architecture 🌟

The relationships between server, handlers and autocomplete in the diagram seems a bit weird as all the data/logic is going back and forth between all of them. My understanding of the logic for this project would be something like:

server <--> router <--> handlers <--> autocomplete logic

Creating a css file

To remove some of the messiness from your handler file it would be better to have a router.js. This would help me to understand the code as well where the router.js is only directing to handlers based on url and the handler.js is handling the data/calling other functions to do so. Please modularise your code.

A handler function should be created in the src/handler.js file so that the server will call each time it gets the request. The handler should initially response to all the requests related to the public files, index.html, css and etc...

Please please sort out indentation. Run atom beautify or prettier to sort it all out for you. This shouldn't be an issue but it makes it harder to read code if things are not indented properly.

Set up the server is it can response to the client-side