Skip to content
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

[SALAD-23967] WebApp: Earning Table - added #1272

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

vitto-moz
Copy link
Contributor

@vitto-moz vitto-moz commented Jan 31, 2025

Task on hold

@vitto-moz vitto-moz self-assigned this Jan 31, 2025
Copy link

@vitto-moz vitto-moz marked this pull request as ready for review February 5, 2025 13:32
@vitto-moz vitto-moz requested a review from a team as a code owner February 5, 2025 13:32

const getRows = (): Array<TableRow> => {
return machines
.filter((machine) => machine.machine_id)
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we need this filter?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

property machine_id is marked as optional by kioto types. I don't think it's a relevant case, but we should handle it

Comment on lines +112 to +113
console.log('daysShowing ===> ', daysShowing)
console.log('earningsPerMachine ===> ', earningsPerMachine)
Copy link
Contributor

Choose a reason for hiding this comment

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

remove?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we have temporary keep them to avoid ts errors. Current implementation is partial and we will need this data in future - so no need to remove these props and related code

@vitto-moz vitto-moz requested a review from rners01 February 7, 2025 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants