-
Notifications
You must be signed in to change notification settings - Fork 36
Updated Project #47
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?
Updated Project #47
Changes from 20 commits
ad2404b
575b160
038f93e
30785ac
96682be
e287191
b4e7f57
1f6c3a3
eef44e5
097c7c1
3e38dff
3c23cb2
d8792ca
55837eb
10b0d40
11ea6c4
2b53415
c15fe4c
90d744b
2361a84
eeb1c81
9580c60
cc6a0dd
f18872b
24aba76
2e7e23d
0d8a9d4
2313df4
817249c
cb7c3ea
f8204e9
249611b
7a4b0e2
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,265 @@ | ||
| import { ajax, css } from "jquery"; | ||
|
|
||
| class Myclass { | ||
| firstNumber: number = 0; | ||
| lastNumber: number = 0; | ||
| backend: string = "http://localhost:2050"; | ||
| resizeTimeout: number = 0; | ||
|
|
||
| /** fetches the number of records from backend */ | ||
| fetchRecordCount(): Promise<number> { | ||
| return fetch(`${this.backend}/recordCount`) | ||
| .then(res => { | ||
| if (!res.ok) { | ||
| throw 'Failed to fetch record count'; | ||
| } | ||
| return res.json(); | ||
| }) | ||
| .catch(err => { | ||
| throw 'Error fetching the record count: ' + err; | ||
| }); | ||
| } | ||
|
|
||
| /** fetches columns from backend */ | ||
| fetchColumns(): Promise<string[]> { | ||
| return fetch(`${this.backend}/columns`) | ||
| .then(res => { | ||
| if (!res.ok) { | ||
| throw 'Failed to fetch columns'; | ||
| } | ||
| return res.json(); | ||
| }) | ||
| .catch(err => { | ||
| throw 'Error fetching columns' + err; | ||
| }); | ||
| } | ||
|
|
||
| /** fetches records from backend */ | ||
| fetchRecords(from: number, to: number): Promise<any[]> { | ||
| return fetch(`${this.backend}/records?from=${from}&to=${to}`) | ||
| .then(res => { | ||
| if (!res.ok) { | ||
| throw "Sorry, there's a problem with the network"; | ||
| } | ||
| return res.json(); | ||
| }) | ||
| .catch(err => { | ||
| throw 'Error fetching records from server ' + err; | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| const myClass = new Myclass(); | ||
|
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. Global variables are bad, please remove them.
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. Also... please... please give it a better name... This name creates the idea that the Dev that made it isn't even aware of what it is used for. |
||
|
|
||
| /** Initializes the table head */ | ||
| function createTable():Promise<string[]> { | ||
| return myClass.fetchColumns() | ||
| .then(columns => { | ||
| for (const col of columns) { | ||
| $(".head").append(`<th>${col}</th>`); | ||
| } | ||
| return columns | ||
| }) | ||
| .catch(err => { | ||
| throw 'Error creating table' + err; | ||
| }); | ||
| } | ||
|
|
||
| /** calculates the number of rows that can fit the screen */ | ||
| const calculatingRows = (): number => { | ||
|
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. ... why make a constant variable that points to a function? You can just make a function. Please change this to just be a normal function. |
||
| const screenHeight = window.innerHeight; | ||
| const availableHeight = screenHeight - 105; | ||
| let rowHeight = 35; | ||
| if (availableHeight <= 0) { | ||
| return 0; | ||
| } else { | ||
| let maxRows = Math.floor(availableHeight / rowHeight); | ||
| return maxRows; | ||
| } | ||
| }; | ||
|
|
||
| /** calls to re-display records when screen is adjusted */ | ||
| function handleResize(recordCount: number) { | ||
| $(window).on('resize', () => { | ||
| clearTimeout(myClass.resizeTimeout); | ||
| myClass.resizeTimeout = setTimeout(async () => { | ||
| displayRecords(recordCount) | ||
| let inputValue = $('#searchInput').val(); | ||
| if (inputValue !== '') { | ||
| await searchRecordsAndResize(recordCount); | ||
| } | ||
| }, 250); | ||
| }); | ||
| } | ||
|
FritzOnFire marked this conversation as resolved.
Outdated
|
||
|
|
||
| /** display records that fit the screen */ | ||
| async function displayRecords(recordCount: number): Promise<void> { | ||
| $('#loader').show(); | ||
| const inputValue = $('#searchInput').val() as number; | ||
|
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. Please use |
||
| const calculatedRows = calculatingRows(); | ||
| const { firstNumber, lastNumber } = calculateFirstAndLastNumbers(calculatedRows, recordCount); | ||
| updateArrowVisibility(firstNumber, lastNumber, recordCount); | ||
| const records = await fetchAndDisplayRecords(firstNumber, lastNumber, inputValue); | ||
| $('#page').empty().append(`Showing record: ${firstNumber} - ${lastNumber}`); | ||
| $('#loader').hide(); | ||
| } | ||
|
|
||
| function calculateFirstAndLastNumbers(calculatedRows: number, recordCount: number) { | ||
|
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. Please specify a return type. |
||
| let firstNumber, lastNumber; | ||
| if (myClass.firstNumber < 0 || myClass.firstNumber > recordCount) { | ||
| firstNumber = 0; | ||
| } else { | ||
| firstNumber = myClass.firstNumber; | ||
| } | ||
| lastNumber = firstNumber + calculatedRows - 1; | ||
| if (lastNumber >= recordCount) { | ||
| lastNumber = recordCount; | ||
| firstNumber = lastNumber - (calculatedRows - 1) | ||
| } | ||
| return { firstNumber, lastNumber }; | ||
| } | ||
|
|
||
| function updateArrowVisibility(firstNumber: number, lastNumber: number, recordCount: number) { | ||
| if (firstNumber === 0) { | ||
| $('.arrow-left').hide(); | ||
| } else { | ||
| $('.arrow-left').show(); | ||
| } | ||
|
|
||
| if (lastNumber >= recordCount) { | ||
| $('.arrow-right').hide(); | ||
| } else { | ||
| $('.arrow-right').show(); | ||
| } | ||
| } | ||
|
|
||
|
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 space |
||
| async function fetchAndDisplayRecords(firstNumber: number, lastNumber: number, inputValue: number) { | ||
| const records = await myClass.fetchRecords(firstNumber, lastNumber); | ||
| const tbody = $("tbody"); | ||
| tbody.empty(); | ||
| for (const record of records) { | ||
| /** creates row for each record*/ | ||
| $("tbody").append(`<tr class="row"></tr>`); | ||
| const lastRow = $(".row:last"); | ||
| for (const value of record) { | ||
| /** assign each record to their column in a specified row */ | ||
| lastRow.append(`<td>${value}</td>`); | ||
| } | ||
| if (record.includes(inputValue)) { | ||
| /** highlights the searched row */ | ||
| lastRow.css('background-color', '#DDC0B4'); | ||
| } | ||
| $("tbody").append(lastRow); | ||
| } | ||
| return records; | ||
| } | ||
|
|
||
| /** recalculates the record range that includes inputValue */ | ||
| async function searchRecordsAndResize(recordCount: number): Promise<void> { | ||
| let inputValue = $('#searchInput').val() as number; | ||
|
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. Please use |
||
| if (inputValue < 0 || inputValue > recordCount) { | ||
| $('.modal').css('display', 'block'); | ||
| $('.content').append(`<p>${inputValue} is not a number within the range. Please try a different number</p>`); | ||
| $('#page').empty().append(`Showing record: ${myClass.firstNumber} - ${myClass.lastNumber}`); | ||
| $('#searchInput').val(''); | ||
| } | ||
| let calculatedRows = calculatingRows(); | ||
| /** divides the calculated max rows in half*/ | ||
|
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. Similar comment about incorrect use of javadoc. |
||
| const halfRange = Math.floor(calculatedRows / 2); | ||
| myClass.firstNumber = Math.max(0, inputValue - halfRange); | ||
| myClass.lastNumber = Math.min(recordCount, myClass.firstNumber + (calculatedRows - 1)); | ||
|
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. If I understand correctly, you show the searched record in the middle of the grid and highlight it. In IMQS we show the most relevant record we're searching for at the top of the grid without formatting. But you can still keep the formatting because it's cool.
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. Soooooo... I can't find an example that @CelesteNaude is talking about... I can only find two examples where we do it the way that you are doing it. But that has a lot to do with the fact that most of the time we filter instead of search. (BTW @CelesteNaude, the two places is user-management, where we highlight and scroll to the user you just edited and the grids on the map, when you click on something on the map, and highlight and scroll to the feature you clicked on) 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. This I remember from the advice that was given to me for my own onboarding project, weird. |
||
| await displayRecords(recordCount); | ||
| } | ||
|
|
||
| /** Navigates to the next set of records */ | ||
| async function rightArrow(recordCount: number): Promise<void> { | ||
| $('#page').empty(); | ||
| $('#searchInput').val(''); | ||
| /** retrieves the last row */ | ||
|
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. Similar comment about incorrect use of javadoc. |
||
| const lastRow = document.querySelector("#recordsTable tbody .row:last-child"); | ||
| /** checks if the last row exists */ | ||
|
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. Similar comment about incorrect use of javadoc. |
||
| if (lastRow) { | ||
| const cells = lastRow.querySelectorAll("td"); | ||
| const lastRecord = []; | ||
| for (const cell of Array.from(cells)) { | ||
| lastRecord.push(cell.textContent || ""); | ||
| } | ||
| /** determines te value in the last row */ | ||
|
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. Similar comment about incorrect use of javadoc. |
||
| const lastID = parseFloat(lastRecord[0]); | ||
|
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. Instead of looping over everything... can't you rather just check the length of |
||
| /** checks if the last value is within range */ | ||
|
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. Similar comment about incorrect use of javadoc. |
||
| if (0 <= lastID && lastID <= (recordCount)) { | ||
|
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 need the () around |
||
| const tbody = $("tbody"); | ||
| tbody.empty(); | ||
| /** calculates the first number of the page */ | ||
|
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. Similar comment about incorrect use of javadoc. |
||
| myClass.firstNumber = lastID + 1; | ||
| let calculatedRows = calculatingRows(); | ||
| /** calculates the last number of the page */ | ||
|
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. Similar comment about incorrect use of javadoc. |
||
| myClass.lastNumber = myClass.firstNumber + (calculatedRows - 1); | ||
| await displayRecords(recordCount); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| async function leftArrow(recordCount: number): Promise<void> { | ||
| $('#page').empty(); | ||
| $('#searchInput').val(''); | ||
| /** retrieves the first row */ | ||
|
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. Similar comment about incorrect use of javadoc. |
||
| const firstRow = document.querySelector("#recordsTable tbody .row:first-child"); | ||
| if (firstRow) { | ||
| const cells = firstRow.querySelectorAll("td"); | ||
| const firstRecord = []; | ||
| for (const cell of Array.from(cells)) { | ||
| firstRecord.push(cell.textContent || ""); | ||
| } | ||
| /** determines te value in the first row */ | ||
|
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. Similar comment about incorrect use of javadoc. |
||
| const firstID = parseFloat(firstRecord[0]); | ||
| if (0 <= firstID && firstID <= (recordCount)) { | ||
|
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 above |
||
| const tbody = $("tbody"); | ||
| tbody.empty(); | ||
| const calculatedRows = calculatingRows(); | ||
| myClass.lastNumber = firstID - 1; | ||
| myClass.firstNumber = myClass.lastNumber - (calculatedRows - 1); | ||
| await displayRecords(recordCount); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| window.onload = () => { | ||
| myClass.fetchRecordCount() | ||
| .then(count => { | ||
| let recordCount: number = count - 1; | ||
|
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. No need to specify the type here, please remove it. |
||
| displayRecords(recordCount); | ||
| handleResize(recordCount) | ||
| searchRecordsAndResize(recordCount) | ||
| $('#btnSearch').on("click", (event) => { | ||
| event.preventDefault(); | ||
| $('#page').empty(); | ||
| searchRecordsAndResize(recordCount); | ||
|
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. You are going to have to catch and handle the error here. |
||
| }); | ||
| $('.arrow-right').on('click', () => { | ||
| rightArrow(recordCount); | ||
| }); | ||
| $('.arrow-left').on('click', () => { | ||
| leftArrow(recordCount); | ||
| }); | ||
| }) | ||
| .catch(err => { | ||
| throw new Error('Error fetching record count' + err); | ||
| }); | ||
| createTable(); | ||
|
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. This function can throw an error, please catch it and handle it.
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 comment on this line is still true. |
||
| $('#closeModalBtn').on("click", () => { | ||
| $('.content').empty(); | ||
| $('.modal').css('display', 'none'); | ||
| }); | ||
| $('#searchInput').on('keydown', (event) => { | ||
| if (event.key === 'e' || event.key === 'E') { | ||
| event.preventDefault(); | ||
| } | ||
| }); | ||
| $('#searchInput').on('input', (event) => { | ||
| const inputValue = $('#searchInput').val() as 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. Please use |
||
| if (inputValue.includes('.')) { | ||
| $('#searchInput').val(inputValue.replace('.', '')); | ||
| } | ||
| }); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,13 +1,44 @@ | ||
| <!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> | ||
| <link rel="stylesheet" href="style.css"> | ||
| <script src="/app.js" ></script> | ||
| </head> | ||
|
|
||
| <body> | ||
| <p>Hello</p> | ||
| <div id="loader"> | ||
| <div class="loader-spinner"></div> | ||
| </div> | ||
|
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. Your formatting is a bit off here. |
||
| <h1 class="heading">Javascript Project</h1> | ||
| <div id="modal" class="modal"> | ||
| <div class="modal-content"> | ||
| <span id="closeModalBtn" class="close">×</span> | ||
| <p class="content"></p> | ||
| </div> | ||
| </div> | ||
| <div class="search-container"> | ||
| <form id="searchForm"> | ||
| <input type="number" id="searchInput" placeholder="Search.." name="search" autocomplete="off"> | ||
| <button id="btnSearch" class="btnSearch" type="submit" >Submit</button> | ||
| </form> | ||
| </div> | ||
| <div class="showRecords"> | ||
| <button class="arrow-right"></button> | ||
| <div id="page"></div> | ||
| <button class="arrow-left"></button> | ||
| </div> | ||
| <table id="recordsTable"> | ||
| <thead> | ||
| <tr class="head"> | ||
| </tr> | ||
| </thead> | ||
| <tbody> | ||
| </tbody> | ||
| </table> | ||
|
|
||
| </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,27 @@ | ||
| { | ||
| "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 -p .", | ||
| "start": "npm run build -- -w" | ||
| }, | ||
| "repository": { | ||
| "type": "git", | ||
| "url": "git+https://github.com/ThokozaniNqwili/onboard-javascript.git" | ||
| }, | ||
| "author": "", | ||
| "license": "ISC", | ||
| "bugs": { | ||
| "url": "https://github.com/ThokozaniNqwili/onboard-javascript/issues" | ||
| }, | ||
| "homepage": "https://github.com/ThokozaniNqwili/onboard-javascript#readme", | ||
| "devDependencies": { | ||
| "typescript": "^3.8.3" | ||
| }, | ||
| "dependencies": { | ||
| "@types/jquery": "^3.5.16" | ||
| } | ||
| } |
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.
Please move the class into a separate file.