Canvas width takes padding on scrollContainer into account #3929
Conversation
… scrollContainer minus the left&right padding on it.
WalkthroughThe pull request introduces modifications to the Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
src/renderer.ts (2)
275-276: LGTM! Consider caching computed styles.The padding calculation is correct and aligns with the PR objective. However, since this calculation is duplicated in the
rendermethod, consider caching the computed styles or extracting the calculation to a private method.+ private getPaddingWidth(): number { + const { paddingLeft, paddingRight } = getComputedStyle(this.scrollContainer) + return parseFloat(paddingLeft) + parseFloat(paddingRight) + } + getWidth(): number { - const { paddingLeft, paddingRight } = getComputedStyle(this.scrollContainer) - return this.scrollContainer.clientWidth - parseFloat(paddingLeft) - parseFloat(paddingRight) + return this.scrollContainer.clientWidth - this.getPaddingWidth() }
640-641: Use the extracted method for consistency.The padding calculation here is identical to the one in
getWidth(). Let's use the extracted method to maintain consistency and improve performance.- const { paddingLeft, paddingRight } = getComputedStyle(this.scrollContainer) - const parentWidth = this.scrollContainer.clientWidth - parseFloat(paddingLeft) - parseFloat(paddingRight) + const parentWidth = this.scrollContainer.clientWidth - this.getPaddingWidth()
| const { paddingLeft, paddingRight } = getComputedStyle(this.scrollContainer) | ||
| return this.scrollContainer.clientWidth - parseFloat(paddingLeft) - parseFloat(paddingRight) |
There was a problem hiding this comment.
Hey, sorry for the delay!
I'll need more time to evaluate how the performance will be affected by this as the width is read a few times.
Set the width of the render canvas to the width of the scrollContainer minus the left&right padding on it.
Short description
Resolves #3892
Implementation details
On two spots in render.ts the paddingLeft and paddingRight of the scrollContainer is subtracted from its clientWidth. Once when setting the canvas size, so it takes up the right amount of space. Second in the
getWidth()function.How to test it
Screenshots
See #3892
You can see it is a little bit wider than the wrappers above it (when completely zoomed out), in this case
2*12px = 24px.Checklist
Summary by CodeRabbit
These updates contribute to a more reliable and visually accurate user experience.