Skip to content

fix/bring back all row calculations#871

Merged
revolist merged 1 commit into
mainfrom
fix/pin-widths
May 26, 2026
Merged

fix/bring back all row calculations#871
revolist merged 1 commit into
mainfrom
fix/pin-widths

Conversation

@revolist

@revolist revolist commented May 26, 2026

Copy link
Copy Markdown
Owner

Summary by CodeRabbit

  • Bug Fixes
    • Improved viewport scroll handling for pinned columns to ensure consistent rendering across different column configurations.

Review Change Stack

@revolist revolist merged commit 2ab2973 into main May 26, 2026
4 of 5 checks passed
@revolist revolist deleted the fix/pin-widths branch May 26, 2026 21:42
@coderabbitai

coderabbitai Bot commented May 26, 2026

Copy link
Copy Markdown

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 1b38fa60-0ae4-460f-9b8a-52d3b5fb1497

📥 Commits

Reviewing files that changed from the base of the PR and between b87041d and 6f40a10.

📒 Files selected for processing (1)
  • src/components/scroll/revogr-viewport-scroll.tsx

Walkthrough

The viewport scroll component's render() method simplifies how it calculates physicalContentWidth. The change removes conditional logic that treated pinned column edge types differently, replacing it with an unconditional call to getContentSize(this.contentWidth, 0).

Changes

Viewport scroll width calculation

Layer / File(s) Summary
Physical content width calculation
src/components/scroll/revogr-viewport-scroll.tsx
physicalContentWidth now computed unconditionally via getContentSize(this.contentWidth, 0), removing prior conditional behavior that checked colPinStart/colPinEnd edge types to determine whether to preserve contentWidth unchanged or use the horizontal scroll container's client width.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • revolist/revogrid#870: Modifies the same physicalContentWidth calculation in viewport scroll's render() method, also addressing how pinned column edge types affect width computation.

Suggested labels

bug

Poem

🐰 With whiskers twitching, logic refined,
Pinned columns fade from the calculating mind,
One path forward, clean and true,
The viewport scroll knows just what to do!

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/pin-widths

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@sonarqubecloud

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6f40a10b20

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

this.contentWidth,
this.horizontalScroll?.clientWidth ?? 0,
);
const physicalContentWidth = getContentSize(this.contentWidth, 0);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve pinned viewport width beyond max scroll guard

Using getContentSize(this.contentWidth, 0) here reintroduces scroll-size compression for pinned sections, because getContentSize clamps physical size to the safeContentSize limit in getScrollDimension. For colPinStart/colPinEnd (which do not horizontally scroll), this truncates the rendered pinned pane when total pinned width exceeds the guard (~15,000,000px), so part of pinned columns becomes unreachable; the previous branch avoided this by using raw contentWidth for pinned types.

Useful? React with 👍 / 👎.

This was referenced May 27, 2026
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.

2 participants