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

Fix row theme and column theme cutting off when entering into blank region of Grid #880

Merged
merged 2 commits into from Jan 24, 2024

Conversation

jassmith
Copy link
Contributor

@jassmith jassmith commented Jan 24, 2024

CleanShot 2024-01-23 at 22 33 13@2x

Previously everything below the last data row would just be white

@@ -159,7 +160,11 @@ export function mergeAndRealizeTheme(theme: Theme, ...overlays: Partial<Theme |
for (const key in overlay) {
// eslint-disable-next-line no-prototype-builtins
if (overlay.hasOwnProperty(key)) {
merged[key] = (overlay as any)[key];
if (key === "bgCell") {
merged[key] = blend(overlay[key] as string, merged[key]);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Without doing this we retain the previous requirement that bgCell always be a solid color. Now only the main theme need be a solid color.

}
}
onCellEdited={setCellValue}
onColumnResize={onColumnResize}
rows={1_000_000}
rows={10}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This makes it easier to see the change

ctx.beginPath();
// render in reverse order because we computed and added the columns last, but they should actually be lower
// priority than the rows.
for (let i = toDraw.length - 1; i >= 0; i--) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The complexity here is just to reduce the number of draw calls. Fewer draw calls === faster

@BrianHung
Copy link
Collaborator

image

Seems to be a missing row when trailingRowOptions and onRowAppended are defined.

}

y += rh;
if (row < rows) y2 = y;
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should perhaps be

if (row < rows - freezeTrailingRows) y2 = y;

since drawing of column overflow should start at the last row with data.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Would also change y2 to be a more descriptive variable like lastRowY or extraRowStartY.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you! Great catch.

@BrianHung
Copy link
Collaborator

LGTM after that fix :)

@jassmith jassmith merged commit a67f54e into main Jan 24, 2024
6 checks passed
@jassmith jassmith deleted the jason/row-col-theme-improvements branch January 24, 2024 22:01
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.

None yet

2 participants