Dev #268
Dev #268
Conversation
web-ui: modularize the pages (calculator, governance missing) web-ui: added block-list page, changed latest-blocks to block-table. web-ui: block details/raw page (WIP) web-ui: updated blocks tab web-ui: fixed the colspan for labels in block-details page. web-ui: fixed the unit-tests. web-ui: paginated the block list table web-ui: updated the blocks api url web-ui: updated the block-table component and blocks api endpoint. web-ui: added i18n to the block-list component. web-ui: moved truncate to utils and added test. web-ui: updated the style for better responsiveness web-ui: updated the dashboard page layout web-ui: Fixed type definition of block.transactions field. web-ui: Made amAgo utility function. web-ui: Added transaction list page. web-ui: Fixed unit test for transactions table. web-ui: Updated the transaction details/raw pages web-ui: Updated the dashboard page ui web-ui: Updated the nodes page ui web-ui: Updated the node stats on the UI. web-ui: Fixed unit test for node-ticker. web-ui: Updated the addresses page ui web-ui: Updated the addresses page ui web-ui: Added calculator page ui web-ui: Updated the calculator page ui web-ui: Updated statistics display values web-ui: Updated coins in masternodes value in the ui web-ui: Updated the ui to for coinsStaking
There was a problem hiding this comment.
Overall I took a quick look to this huge change, I'd like to merge it as soon as possible, I'll approve when the PR gets these changes:
- Enable the tests and get them passing in the CI
- Format the code properly, there are some html pieces not aligned.
- Remove unused/commented code
- There are some critical pieces that I want you tor write tests for, like the calculator functions
- Try to avoid using
anytypes, and specify types when possible.
Thanks.
| @@ -4,14 +4,17 @@ | |||
| "license": "MIT", | |||
| "scripts": { | |||
| "ng": "ng", | |||
| "start": "ng serve", | |||
| "serve": "ng serve", | |||
| "start": "node server.js", | |||
There was a problem hiding this comment.
Please explain what's the usefulness to include express because angular already has an embedded server for dev purposes, if this is not necessary, remove it
There was a problem hiding this comment.
That's for staging server. (http://staging.xsnexplorer.io/)
In the staging server, I build the project and run it with node server.
You can check that in server.js.
There was a problem hiding this comment.
In my opinion, this is not necessary for the project, I'd suggest keeping this change in your local machine only
There was a problem hiding this comment.
In fact, given that it is staging, why can't you use ng server instead?
| tposContracts: Array<TposContract>; | ||
| selectedTpos: number; | ||
| isLoading: boolean; | ||
| loadingType = 2; |
There was a problem hiding this comment.
Use a enum instead of int values, or at least, create constants representing the possible statuses instead of using magic numbers
web-ui/src/app/components/addresses/address-details/address-details.component.ts
Outdated
Show resolved
Hide resolved
| addressLabel = addressLabels; | ||
| tposContracts: Array<TposContract>; | ||
| selectedTpos: number; | ||
| isLoading: boolean; |
There was a problem hiding this comment.
you likely need a isLoading per component loaded asynchronously, otherwise, there will be conflicts while loading transactions and tpos contracts
web-ui/src/app/components/addresses/address-details/address-details.component.ts
Outdated
Show resolved
Hide resolved
| templateUrl: './calculator.component.html', | ||
| styleUrls: ['./calculator.component.css'] | ||
| }) | ||
| export class CalculatorComponent implements OnInit, AfterViewChecked { |
There was a problem hiding this comment.
This component has quite a lot of functions that could be taken out of the component, make sure to tests those, its pretty important to get the correct data for users
| <div class="tab-content row ml-0 mr-0"> | ||
| <div class="col-xs-12 text-primary page-title"> | ||
| {{ip ? ip : 'label.masternode' | translate}} | ||
| </div> |
There was a problem hiding this comment.
Please make sure that all the changes have properly formatted code
pull-request-attention
bot
assigned midasdev711 and AlexITC and unassigned AlexITC and midasdev711
pull-request-attention
bot
assigned midasdev711 and AlexITC and unassigned AlexITC and midasdev711
|
Superseded by #329 . |
Problem
Solution
Result
All issues are fixed.