-
Notifications
You must be signed in to change notification settings - Fork 722
add more parameters to TableView's renderCell function #4254
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
base: v2_develop
Are you sure you want to change the base?
add more parameters to TableView's renderCell function #4254
Conversation
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.
Pull Request Overview
This PR enhances the TableView's cell rendering functionality by adding row and column position parameters to the RenderCell method. This change enables developers to know which specific cell is being rendered when overriding the cell rendering behavior.
- Added three new parameters (
row
,rowToRender
,columnToRender
) to theRenderCell
method signature - Updated the method call in
RenderRow
to pass the additional position parameters - Updated the example override implementation to match the new signature
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
Terminal.Gui/Views/TableView/TableView.cs | Enhanced RenderCell method signature with position parameters and updated the method call |
Examples/UICatalog/Scenarios/MultiColouredTable.cs | Updated override implementation to match new RenderCell signature |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
/// <param name="cellAttribute"></param> | ||
/// <param name="render"></param> | ||
/// <param name="isPrimaryCell"></param> | ||
protected virtual void RenderCell (Attribute cellAttribute, string render, bool isPrimaryCell) | ||
/// <param name="row"></param> | ||
/// <param name="rowToRender"></param> | ||
/// <param name="columnToRender"></param> |
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.
The parameter documentation is missing descriptions. Consider adding meaningful descriptions like /// <param name="row">The logical row index in the data source</param>
, /// <param name="rowToRender">The visual row position being rendered</param>
, and /// <param name="columnToRender">The column index being rendered</param>
.
Copilot uses AI. Check for mistakes.
Hmn... for v2 we should probably prioritise using the events system for this. Add a DrawingCell event for example. That would be preferable to override I think. @tig thoughts? @metasong would you be up for having a go at this? I can assist Benefits include:
Proposition would be to make it a cancelable event. User would set cancel to true and call relevant methods to draw themselves |
I concur. |
when override the cell rendering, we do not know which cell is rendering