Gerald - Onboarding #38
Conversation
Create main.css Create get httprequest.
…for grid headings and records.
Foundational setup
Create http request in typescript. Update css, and data table.
Convert data to be usable for functions.
Use data from server
render table records and headings to html
Refactor code and add comments
…uide Edit code to meet code style requirements
fix navigation
Code refactoring
|
Please remove your |
I removed the files and changed gitignore file. |
FritzOnFire
left a comment
FritzOnFire
left a comment
There was a problem hiding this comment.
You have a lot of variables and statements executing in global space which is generally frown'd upon. For this project, there are a few that we are ok with being in global space, but you are running a lot of them without waiting for the page to even finish loading.
Reworked global variables
combine code in app.ts script
FritzOnFire
left a comment
FritzOnFire
left a comment
There was a problem hiding this comment.
Please read up on window.onload. All of this global space statements are really bothering me. Please move them somewhere more sane. Please change all of your comments for functions and classes to JavaDoc style. Please do not use an interface before you have defined it (HasFormatMethod) and finally, it really looks like your formatter is not kicking in. I will send you my vscode settings incase you can not get your formatting working.
app.ts
Outdated
| let totalNumofRecords: number; | ||
| let clickTimeout: ReturnType<typeof setTimeout>; | ||
| let fromID = 1; | ||
| let toID = numOfRows; |
There was a problem hiding this comment.
I dont think all of these need to be global, especially numofrows
nkosimarz
left a comment
nkosimarz
left a comment
There was a problem hiding this comment.
The structure is definitely cleaner, LGTM
|
|
||
| /** | ||
| * This event executes two funtions immediately after the page loads | ||
| * @function HFM.getColumnHeadings This is a function that gets the column headings from the back-end |
There was a problem hiding this comment.
I prefer this pattern for comments: @function HFM.getColumnHeadings gets the column heading ...
There was a problem hiding this comment.
Thanks. I'll quickly change the comments to your suggestion
4 participants
No description provided.