-
Notifications
You must be signed in to change notification settings - Fork 3
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
fix: compliance report fuel supply end use logic #1587
Conversation
} | ||
} | ||
|
||
if (params.column.colId === 'fuelCategory') { |
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.
Do you think its cleaner for column specific logic to live in a value setter or value getter for that column rather than a big if else in handle cell changed? I believe we kinda have a mix of both.
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.
Yea I was debating this as well. The thing I don't like about ag grid is that they don't provide a clean validation schema that's separated from row/column/cell logic. I'm going to look to rework some of this logic in a refactor ticket
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.
LGTM 👍
note: will need to add the new seed values to migration in #1503 pr: #1565