-
Notifications
You must be signed in to change notification settings - Fork 118
update: adjust row height on font size change #613
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: main
Are you sure you want to change the base?
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 adds automatic row height adjustment when font size changes in spreadsheet cells. Previously, increasing font size could cause text overflow, requiring manual row resizing. The implementation captures cell state before font size updates, calculates required heights based on the new font sizes and multi-line content, and applies the adjustments.
Key Changes
- Added logic to automatically adjust row heights when font size increases or decreases
- Implemented support for multi-line cells (created with Alt/Option+Enter) in height calculations
- Added range normalization to handle selections made in any direction
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const lineHeight = newFontSize * 1.5; | ||
| const requiredHeight = (maxLineCount - 1) * lineHeight + 8 + newFontSize; |
Copilot
AI
Dec 7, 2025
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 row height calculation doesn't consider the wrap_text style property. In worksheetCanvas.ts:1016-1021, the autofit logic uses computeWrappedLines() to properly calculate line count based on cell width and wrap settings. Without considering wrap_text, this implementation may calculate incorrect heights for cells with text wrapping enabled, potentially causing text to overflow or excess whitespace.
| newRowHeight = DEFAULT_ROW_HEIGHT; | ||
| } | ||
|
|
||
| if (Math.abs(newRowHeight - oldRowHeight) > 0.1) { |
Copilot
AI
Dec 7, 2025
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 threshold 0.1 is a magic number without explanation. Consider extracting it as a named constant (e.g., MIN_HEIGHT_CHANGE_THRESHOLD = 0.1) with a comment explaining why this epsilon is needed (likely to avoid unnecessary updates for floating-point precision differences).
| let newRowHeight: number; | ||
| if (requiredHeight > DEFAULT_ROW_HEIGHT) { | ||
| newRowHeight = requiredHeight; | ||
| } else { | ||
| newRowHeight = DEFAULT_ROW_HEIGHT; | ||
| } |
Copilot
AI
Dec 7, 2025
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.
[nitpick] The variable newRowHeight is unnecessary since it's only assigned once and immediately used. Consider simplifying by directly assigning to a const:
const newRowHeight = requiredHeight > DEFAULT_ROW_HEIGHT ? requiredHeight : DEFAULT_ROW_HEIGHT;This makes the code more concise and easier to read.
| let newRowHeight: number; | |
| if (requiredHeight > DEFAULT_ROW_HEIGHT) { | |
| newRowHeight = requiredHeight; | |
| } else { | |
| newRowHeight = DEFAULT_ROW_HEIGHT; | |
| } | |
| const newRowHeight = requiredHeight > DEFAULT_ROW_HEIGHT ? requiredHeight : DEFAULT_ROW_HEIGHT; |
This PR ensures that row heights automatically adapt when the font size changes.
Previously, increasing the font size could cause text to overflow or become clipped, requiring manual row resizing.
The update introduces a smarter height adjustment mechanism and fixes issues with multi-line cell content.
Changes
font.resizing.mov
Testing