For Review#3
Conversation
Velink
commented
Sep 9, 2021
Velink
commented
- Apologies for the reverse merge
Development
Revert "Development"
dimtaiwo
left a comment
dimtaiwo
left a comment
There was a problem hiding this comment.
-Awesome UI
- Good implementation
- some tests are still failing
- too much stuffs still logged in the console
- on the client-side, only one button working properly
- on script Js, I think exporting the module was unnecessary
- Search results are well sorted.
- Overall Great Job, I love it !!! 😍
brodie-m
left a comment
brodie-m
left a comment
There was a problem hiding this comment.
Really nice job guys, particularly enjoyed how you took on the challenge of making your own search engine
- UI was spot on -enjoyed the attention to detail with displaying the results. Would like to see a separate results page and some more responsive elements
- In some places I feel that the CSS could be simplified (but it looks exactly like google, so well done!) - would like to see colours defined as variables to allow for easier styling of the whole page
- JS is nicely formatted and very clean - maybe some clearer variable names would make it more readable.
- some cases are still hard-coded (e.g., for loop with i <= 9)
- I like the implementation of the search function, but would like to see fewer results displayed when searching for something specific, or when searching for nothing at all
- Could add lucky button quite easily by just returning the first result from the search function
- Lots of console.logs left in which don't matter now, but this will when the results array is much larger
- Would like to see some code refactoring + MVC + all tests passing
| Challenges: | ||
|
|
||
| - Figuring out how to sort our data objects based on the user input | ||
| - Looping through the user input array and our data array, and returning a result which is soted from most relevant to least relevant No newline at end of file |
There was a problem hiding this comment.
nice readme - very helpful - would like to see some formatting
| </div> | ||
|
|
||
| </body> | ||
| </html> No newline at end of file |
| z-index: 20; | ||
| } | ||
|
|
||
| #nav-ul-2 li:nth-of-type(3) a:hover::after{ |
| #footer-ul-2 li:hover{ | ||
| cursor: pointer; | ||
| text-decoration: underline; | ||
| } No newline at end of file |
There was a problem hiding this comment.
really nice job on the styling - would like to see some more responsive elements, e.g., results page is kind of broken upon resizing
maybe define some variables for the reused colours
| .expect(200) | ||
| .expect({id: 2 , link: "", title: "" , description: ""}, done) | ||
| }) | ||
| }) No newline at end of file |
There was a problem hiding this comment.
nice testing - i like how you left in the same console.logs from the demo
final test expecting the wrong thing?
There was a problem hiding this comment.
lol forgot to change this after I wrote the test at the start before the objects had info
| res.send('linkPage') | ||
| }) | ||
|
|
||
| module.exports = app; No newline at end of file |
There was a problem hiding this comment.
nicely done - would be even better with refactoring into MVC
| let str2 = resData[i]["title"].toLowerCase(); | ||
|
|
||
| if (str.includes(word.toLowerCase())) { | ||
| console.log('butter'); |
There was a problem hiding this comment.
is this working as intended? e.g., if I search 'netflixd', should it return netflix as the top result?
| // Append to our link container each link object with the class for that styling | ||
|
|
||
| function orderArray(array) { | ||
| let sortedArray = array.sort((a,b) => b['count'] > a['count'] ? 1 : -1); |
There was a problem hiding this comment.
this is cool - you could maybe use the same count system to do search history
4 participants
