Skip to content
Open
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
3a4e462
update
MaxTF141 Aug 14, 2023
8a555a7
Fixed the table filling the entire screen and started on the search. …
MaxTF141 Aug 14, 2023
75dc47a
Added a new function that can load records based on the screen size. …
MaxTF141 Aug 15, 2023
476cad9
Updated functionality to display number of records according to heigh…
MaxTF141 Aug 16, 2023
b11aad3
Fixed resizing issue by putting a debounce delay on the window resize.
MaxTF141 Aug 16, 2023
6d8b880
updated my pagination in relation to the search function
MaxTF141 Aug 17, 2023
ecf387c
During resizing this version of the project change the start and end …
MaxTF141 Aug 18, 2023
441ca1d
updated. When adjusting height the first record of the page stays the…
MaxTF141 Aug 18, 2023
c948341
update
MaxTF141 Aug 21, 2023
700d931
Merge branch 'master' of github.com:MaxTF141/onboard-javascript into …
MaxTF141 Aug 22, 2023
090f7cd
updated
MaxTF141 Aug 22, 2023
9005d45
Fixed my last value's page and the pagination when going back and for…
MaxTF141 Aug 23, 2023
86fb0dd
Fixed my paginapages when resizing and starting id's on smaller numbe…
MaxTF141 Aug 24, 2023
3b5990f
Formatted
MaxTF141 Aug 28, 2023
db01e42
Almost done with the requested changes
MaxTF141 Aug 28, 2023
2764976
Restructured my global variables and changed it to properties.
MaxTF141 Aug 29, 2023
df647ea
Added semicolons
MaxTF141 Aug 30, 2023
f8f8d24
updated the search input to not take letter and symbols
MaxTF141 Aug 30, 2023
44b81a7
Added user only allowing to type letters it the search bar and a ente…
MaxTF141 Aug 31, 2023
ab121b8
Small changes
MaxTF141 Sep 1, 2023
0f4c00b
Added a Class and are handling all my api calls in the Class. And did…
MaxTF141 Sep 4, 2023
088d73b
Moved my class to a seperate file and replaced try, catch with .then(…
MaxTF141 Sep 6, 2023
975bc24
Update
MaxTF141 Sep 7, 2023
3fb7c44
update
MaxTF141 Sep 11, 2023
ad77763
Delete apiManager.js
MaxTF141 Sep 11, 2023
8704cb7
Delete ApiManager.js
MaxTF141 Sep 11, 2023
cff0b57
Delete apiManager.js.map
MaxTF141 Sep 11, 2023
3198ed0
Delete ApiManager.js.map
MaxTF141 Sep 11, 2023
530cef4
removed js files
MaxTF141 Sep 11, 2023
48d8158
Merge branch 'master' of github.com:MaxTF141/onboard-javascript
MaxTF141 Sep 11, 2023
53ca9e5
requested changes still in progress
MaxTF141 Sep 13, 2023
765552d
set starting value of records to zero and made small requested change…
MaxTF141 Sep 14, 2023
59da1fc
Did the requested changes.
MaxTF141 Sep 15, 2023
d9882f8
update
MaxTF141 Sep 15, 2023
18ee383
Did the requested changes
MaxTF141 Sep 20, 2023
ff94cd8
small update
MaxTF141 Sep 20, 2023
99ca15c
Did most of the requested changes
MaxTF141 Sep 22, 2023
12164f0
Updated calculations for more precision
MaxTF141 Sep 26, 2023
43bef10
Updated comments.
MaxTF141 Sep 26, 2023
9b9bcc6
Did all the requested changes including error handling
MaxTF141 Sep 29, 2023
e2e0439
Update
MaxTF141 Sep 29, 2023
1d6c7ba
Update
MaxTF141 Oct 2, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions .vscode/launch.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
{
"version": "0.2.0",
"configurations": [
{
"type": "chrome",
"request": "launch",
"name": "Debug in Chrome",
"url": "http://localhost:2050/",
"webRoot": "${workspaceFolder}",
"sourceMapPathOverrides": {
"webpack:///./*": "${webRoot}/*"
}
}
]
}
15 changes: 15 additions & 0 deletions .vscode/tasks.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
{
"version": "2.0.0",
"tasks": [
{
"type": "typescript",
"tsconfig": "tsconfig.json",
"option": "watch",
"problemMatcher": [
"$tsc-watch"
],
"group": "build",
"label": "tsc: watch - tsconfig.json"
}
]
}
28 changes: 28 additions & 0 deletions apiManager.js
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't add your .js files to this PR.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions apiManager.js.map
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't add your .js.map files to this PR.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

30 changes: 30 additions & 0 deletions apiManager.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
export class ApiManager {
private mainUrl: string;
Comment on lines +1 to +3
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Space inbetween.

constructor(mainUrl: string) {
Comment on lines +2 to +5
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please leave an empty line between your class variables and constructor

this.mainUrl = mainUrl;
}

private fetchJson(mainUrl: string): Promise<any> {
return fetch(mainUrl)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
private fetchJson(mainUrl: string): Promise<any> {
return fetch(mainUrl)
private fetchJson(url: string): Promise<any> {
return fetch(url)

Feels incorrect calling it the main one since in this scope it is only one and can be anything. Main or not.

.then(res => res.text())
.then(data => JSON.parse(data))
.catch((err) => {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Either remove the () around err or specify a type

throw err
})
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to check if your fetch was successful, as that does not get catch'd by the .catch.

Please use res.json() instead of res.text() and then JSON.parse(). (I know someone else asked you to do it this way, and I apologize that you have to revert your code now again, but this is the right way)

Also, please add a ; to the throw and at the end.

}

// This function will handle retrieving the records from the api
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please change this into a javadoc style comment (This is, sadly, currently not in our style guide.)

Also, no need to say a function is a function. Rather just focus on the action.
bad eg:

/**
 * This function will punch a wall in the game world
 * @param x horizontal coordinate of wall
 * @param y vertical coordinate of wall
 * @returns true if wall was successfully punched
 */
punchWall(x: number, y: number): boolean

Good eg:

/**
 * Punches a wall
 * @param x horizontal coordinate of wall
 * @param y vertical coordinate of wall
 * @returns true if wall was successfully punched
 */
punchWall(x: number, y: number): boolean

getRecords(fromID: number, toID: number): Promise<string[][]> {
return this.fetchJson(`${this.mainUrl}records?from=${fromID}&to=${toID}`);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return this.fetchJson(`${this.mainUrl}records?from=${fromID}&to=${toID}`);
return this.fetchJson(`${this.mainUrl}/records?from=${fromID}&to=${toID}`);

}

// This function will handle retrieving the columns from the api
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment as before about javadoc style.

getColumns(): Promise<string[]> {
return this.fetchJson(`${this.mainUrl}columns`);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return this.fetchJson(`${this.mainUrl}columns`);
return this.fetchJson(`${this.mainUrl}/columns`);

}

// This function will handle retrieving the record count from the api
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same function as before about javadoc style.

getRecordCount(): Promise<number> {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I seriously want to ask you to cache the record count result.

It is the type of call you can make once and trust that it won't change again.
It is a different case if you are working with live data but in your case, you are not.
I would've made that assumption. You can make the fetch once and save it when this class gets instantiated. Then return the result with getRecordCount when you need it. Less chatting to your backend.

However, there is nothing wrong with what you did, as this is a good approach in some cases.

return this.fetchJson(`${this.mainUrl}recordCount`);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return this.fetchJson(`${this.mainUrl}recordCount`);
return this.fetchJson(`${this.mainUrl}/recordCount`);

}
Comment on lines +34 to +49
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So /recordCount returns a number as text... not a json... this feels misleading. You should probably do a different fetch to handle this case.

}
Loading