-
Notifications
You must be signed in to change notification settings - Fork 36
Devin Fledermaus #44
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
base: master
Are you sure you want to change the base?
Devin Fledermaus #44
Changes from 26 commits
88a0c43
69a305f
95cabc6
5f81214
75bff11
3b59290
6de1c55
7bd7bc8
7991e74
c54940f
3e5eae5
b028cc4
dfc1c75
6c53b8d
6a9233b
708034d
027f290
c4039ef
ecceaf0
5ff3563
405592d
c67502c
7cf9e78
708ef31
1256e0c
93e2511
0b1c2e6
876307b
d2cf1df
e31c2d3
f5fdd2d
951158c
46fc0eb
b581516
4d5c1a5
f2457da
ade9748
cdaf8aa
b72ecd9
7b4e7a6
62417f3
d3ad477
e82ac48
4bdde48
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,277 @@ | ||
| // Function To Get Number Of Rows That Can Be Displayed While Still Being Readable | ||
|
|
||
| const getNoOfRows = () => { | ||
| const height = window.innerHeight; | ||
|
|
||
| let number = height / 40; | ||
| let noOfRows = Math.floor(number); | ||
| return noOfRows; | ||
| }; | ||
|
|
||
| // Variables | ||
|
|
||
| let paramOne = 0; | ||
| let paramTwo = paramOne + getNoOfRows(); | ||
|
|
||
| //// Functions To Create/Clear The HTML | ||
|
|
||
| // Heading Row | ||
|
|
||
| const createHeadingRow = (headingData: string) => { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rather remove the empty line between the variable and the comment. |
||
| const heading: any = document.querySelector("#heading"); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The only place you can have an |
||
|
|
||
| let headings = `<div class="headings" id="headings">${headingData}</div>`; | ||
| heading.innerHTML += headings; | ||
| }; | ||
|
|
||
| // Table Content | ||
|
|
||
| const createTableContent = (contentData: string) => { | ||
| const content: any = document.querySelector("#content"); | ||
|
|
||
| let table = `<div id="row-${contentData[0]}" class="rows"></div>`; | ||
| content.innerHTML += table; | ||
|
|
||
| let rows: any = document.querySelector("#row-" + contentData[0] + ".rows"); | ||
| for (let x = 0; x < contentData.length; x++) { | ||
| let rowCols = `<div class="row_cols">${contentData[x]}</div>`; | ||
| rows.innerHTML += rowCols; | ||
| } | ||
| }; | ||
|
|
||
| // Clear Table Content | ||
|
|
||
| const clearTable = () => { | ||
| const content: any = document.querySelector("#content"); | ||
| const clear = ""; | ||
|
|
||
| content.innerHTML = clear; | ||
| }; | ||
|
|
||
| //// Fetch Requests | ||
|
|
||
| // Heading Row (Getting the columns data) | ||
|
|
||
| const getHeadings = () => { | ||
| try { | ||
| fetch("http://localhost:2050/columns", { | ||
| method: "GET", | ||
| headers: { "Content-Type": "application/json" }, | ||
| }) | ||
| .then((res) => res.text()) | ||
| .then((data) => { | ||
| data = JSON.parse(data); | ||
| let headingData = data; | ||
| for (let i = 0; i < headingData.length; i++) { | ||
| createHeadingRow(headingData[i]); | ||
| } | ||
| }); | ||
| } catch (error) { | ||
| console.log(error); | ||
| } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I meant, a catch on the then of the Promise. Not a seperate try and catch block. |
||
| }; | ||
|
|
||
| // Table Content (Getting the table's data) | ||
|
|
||
| const getTable = () => { | ||
| try { | ||
| fetch("http://localhost:2050/records?from=" + paramOne + "&to=" + paramTwo, { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd like you to change this function to accept the from and to variables as parameters. We don't like global variables if not necessary. Also give these variables more descriptive names. Give paramOne and paramTwo names more meaning to what they really are. |
||
| method: "GET", | ||
| headers: { "Content-Type": "application/json" }, | ||
| }) | ||
| .then((res) => res.text()) | ||
| .then((data) => { | ||
| data = JSON.parse(data); | ||
| let contentData = data; | ||
| for (let i = 0; i < contentData.length; i++) { | ||
| createTableContent(contentData[i]); | ||
| } | ||
| }); | ||
| } catch (error) { | ||
| console.log(error); | ||
| } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as the above comment on the catch |
||
| }; | ||
|
|
||
| // Displays The Current Results Being Shown | ||
|
|
||
| const stats = () => { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In general, this function should not have to do a recordCount API call. The recordCount is constant throughout in this case. You can probably just build your string in here and update it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What I mean by recordCount is constant is that you don't have to make an API call every time you call stats since the value is going to be the same for each call. In this case, I recommend you create a global const variable that you assign the recordCount API response once on the initial load of your program. At the moment it is a hardcoded value. Let the backend determine it. |
||
| const pageStats: any = document.querySelector("#pageStats"); | ||
|
|
||
| try { | ||
| fetch("http://localhost:2050/recordCount", { | ||
| method: "GET", | ||
| headers: { "Content-Type": "application/json" }, | ||
| }) | ||
| .then((res) => res.text()) | ||
| .then((data) => { | ||
| data = JSON.parse(data); | ||
| let count = data; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably not necessary to do this over 2 lines. let count = JSON.parse(data); |
||
| let currentStats = "Showing results from " + paramOne + " to " + paramTwo + " out of " + count + " results."; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rather use Template Literals. ${} |
||
| pageStats.innerHTML = currentStats; | ||
| }); | ||
| } catch (error) {} | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as the above comment on the catch |
||
| }; | ||
|
|
||
| //// Debounce | ||
|
|
||
| const debounce = (fn: any, delay: number) => { | ||
| let timer: number; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You might need to add timer as a global variable. It's something I noticed with Ashton as well and he got it to work. At this stage, timer is only in the scope of this function. |
||
| return function () { | ||
| clearTimeout(timer); | ||
| timer = setTimeout(() => { | ||
| fn(); | ||
| }, delay); | ||
| }; | ||
| }; | ||
|
|
||
| //// Sizing And Resizing | ||
|
|
||
| let resizing = () => { | ||
| let end = paramOne + getNoOfRows(); | ||
|
|
||
| if (end > 999999) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't use 999999. You have recordCount for the max.. |
||
| paramTwo = 999999; | ||
| paramOne = paramTwo - getNoOfRows(); | ||
| } else { | ||
| paramOne; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove redundancy. |
||
| paramTwo = paramOne + getNoOfRows(); | ||
| } | ||
| clearTable(); | ||
| getTable(); | ||
| stats(); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can definitely see these 3 functions actually being in one as it always goes together logically. They are always repeated in that order and are not too complex to be split up. It can be all together in getTable()? |
||
| }; | ||
|
|
||
| resizing = debounce(resizing, 500); | ||
|
|
||
| window.addEventListener("resize", resizing); | ||
|
|
||
| //// On Window Load | ||
|
|
||
| window.onload = function () { | ||
| getHeadings(); | ||
| getTable(); | ||
| stats(); | ||
| }; | ||
|
|
||
| //// Navigation | ||
|
|
||
| // Next | ||
| const nextButton: any = document.querySelector("#next"); | ||
| let nextCount = 0; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm struggling to understand why you need these count variables? They look like they are kinda contradicting the point of the debouncing?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm using it to get the amount of times the button is being clicked so i can move next according to the amount |
||
|
|
||
| const nextDebounce = (fn: any, delay: number) => { | ||
| let timer: any; | ||
| return function () { | ||
| nextCount++; | ||
| clearTimeout(timer); | ||
| timer = setTimeout(() => { | ||
| fn(); | ||
| }, delay); | ||
| }; | ||
| }; | ||
|
|
||
| let next = () => { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The paramOne and paramTwo should be accepted as parameters to this function. |
||
| if (paramTwo === 999999) { | ||
| alert("You have reached the final page"); | ||
| } | ||
|
|
||
| let nextAmount = paramTwo - paramOne + 1; | ||
| let nextCountAmount = nextAmount * nextCount; | ||
| paramOne = paramOne + nextCountAmount; | ||
| paramTwo = paramOne + getNoOfRows(); | ||
|
|
||
| let end = paramOne + getNoOfRows(); | ||
|
|
||
| if (end > 999999) { | ||
| paramTwo = 999999; | ||
| paramOne = paramTwo - getNoOfRows(); | ||
| } | ||
|
|
||
| nextCount = 0; | ||
|
|
||
| clearTable(); | ||
| getTable(); | ||
| stats(); | ||
| }; | ||
|
|
||
| next = nextDebounce(next, 500); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You should not have to do this. Just pass the nextDebounce in directly to the eventListener in this case. |
||
|
|
||
| nextButton.addEventListener("click", next); | ||
|
|
||
| // Previous | ||
| const prevButton: any = document.querySelector("#prev"); | ||
| let prevCount = 0; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as the above comment for the next |
||
|
|
||
| const prevDebounce = (fn: any, delay: number) => { | ||
| let timer: any; | ||
| return function () { | ||
| prevCount++; | ||
| clearTimeout(timer); | ||
| timer = setTimeout(() => { | ||
| fn(); | ||
| }, delay); | ||
| }; | ||
| }; | ||
|
|
||
| let prev = () => { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as mentioned in the next. |
||
| if (paramOne === 0) { | ||
| alert("You Are On The First Page"); | ||
| } else { | ||
| let prevAmount = paramTwo - paramOne + 1; | ||
| let prevCountAmount = prevAmount * prevCount; | ||
|
|
||
| let intOne = paramOne - prevCountAmount; | ||
|
|
||
| if (intOne < 0) { | ||
| paramOne = 0; | ||
| } else { | ||
| paramOne = intOne; | ||
| } | ||
|
|
||
| paramTwo = paramOne + getNoOfRows(); | ||
|
|
||
| prevCount = 0; | ||
|
|
||
| clearTable(); | ||
| getTable(); | ||
| stats(); | ||
| } | ||
| }; | ||
|
|
||
| prev = prevDebounce(prev, 500); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same vibe as the above next |
||
|
|
||
| prevButton.addEventListener("click", prev); | ||
|
|
||
| // ID Jump | ||
| const input: any = document.querySelector("input"); | ||
|
|
||
| let idJump = () => { | ||
| let currentID = paramOne; | ||
| let search = input.value; | ||
| let end = parseInt(search) + getNoOfRows(); | ||
|
|
||
| if (search !== NaN && search !== "" && search < 1000000 && search >= 0) { | ||
| if (end > 999999) { | ||
| paramTwo = 999999; | ||
| paramOne = paramTwo - getNoOfRows(); | ||
| } else { | ||
| paramOne = parseInt(search); | ||
| paramTwo = paramOne + getNoOfRows(); | ||
| } | ||
| } else if (search === "") { | ||
| //pass | ||
| } else { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Try to never implement an if-else statement that does nothing. Remove the last else and then you can change your logic for the last if-else like (search !== "") because that is in essence what the logic is for the last else in your case. |
||
| alert("Make Sure Your Desired ID Is Not A Negative Number Or Doesn't Exceed 999999"); | ||
| paramOne = currentID; | ||
| paramTwo = paramOne + getNoOfRows(); | ||
| input.value = ""; | ||
| } | ||
|
|
||
| clearTable(); | ||
| stats(); | ||
| getTable(); | ||
| }; | ||
|
|
||
| idJump = debounce(idJump, 500); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same vibe as the above next. |
||
|
|
||
| window.addEventListener("input", idJump); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,13 +1,27 @@ | ||
| <!DOCTYPE html> | ||
| <html> | ||
| <head> | ||
| <title>JS Onboard Project</title> | ||
| <script type="text/javascript" charset="utf-8" src="third_party/jquery-2.0.3.min.js"></script> | ||
| </head> | ||
|
|
||
| <body> | ||
| <p>Hello</p> | ||
| </body> | ||
|
|
||
| <html lang="en"> | ||
| <head> | ||
| <meta charset="UTF-8" /> | ||
| <meta http-equiv="X-UA-Compatible" content="IE=edge" /> | ||
| <meta name="viewport" content="width=device-width, initial-scale=1.0" /> | ||
| <title>Onboarding JavaScript Task</title> | ||
| <script type="text/javascript" charset="utf-8" src="third_party/jquery-2.0.3.min.js"></script> | ||
| <link rel="stylesheet" href="style.css" /> | ||
| <script src="app.js" defer></script> | ||
| </head> | ||
| <body> | ||
| <div id="nav"> | ||
| <label id="pageStats"></label> | ||
| <div id="jumpID"> | ||
| <input type="number" id="idJump" placeholder="Input A Starting ID" /> | ||
| <!-- <button>Jump to ID</button> --> | ||
| </div> | ||
| <div id="btns"> | ||
| <button class="prev" id="prev">Prev</button> | ||
| <button class="next" id="next">Next</button> | ||
| </div> | ||
| </div> | ||
| <div id="heading"></div> | ||
| <div id="content"></div> | ||
| </body> | ||
| </html> | ||
|
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| { | ||
| "name": "onboard-javascript", | ||
| "version": "1.0.0", | ||
| "description": "This is a JavaScript project for all new developers to complete before venturing into our web frontend codebase.", | ||
| "main": "index.js", | ||
| "scripts": { | ||
| "test": "echo \"Error: no test specified\" && exit 1", | ||
| "build": "tsc --build" | ||
| }, | ||
| "repository": { | ||
| "type": "git", | ||
| "url": "git+https://github.com/Koumori97/onboard-javascript.git" | ||
| }, | ||
| "author": "", | ||
| "license": "ISC", | ||
| "bugs": { | ||
| "url": "https://github.com/Koumori97/onboard-javascript/issues" | ||
| }, | ||
| "homepage": "https://github.com/Koumori97/onboard-javascript#readme", | ||
| "dependencies": { | ||
| "typescript": "^4.6.3" | ||
| }, | ||
| "devDependencies": { | ||
| "@types/jquery": "^3.5.14" | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add return type