Skip to content

fix(pivot-table): add bottom border to last grouped row #36081

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

fix(pivot-table): add bottom border to last grouped row #36081

wants to merge 5 commits into from

Conversation

@igor-tech
Copy link

@igor-tech igor-tech commented Nov 12, 2025

SUMMARY

Fixes missing bottom border on the last row label when grouping is expanded in Pivot Table visualization.

Previously, when multiple groups were present and the last group was expanded, the bottom border was not rendered, creating a visual inconsistency. This PR adds logic to detect the last row and applies the pvtRowLabelLast CSS class, which adds the missing border-bottom style.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Before:
image

After:
image

TESTING INSTRUCTIONS

  1. Create a Pivot Table chart with multiple row groupings
  2. Expand all groups to show nested data
  3. Verify that the bottom border is now visible on the last row label
  4. Check that the border appears correctly when:
    • Last group is collapsed
    • Last group is expanded
    • Column totals are enabled/disabled

ADDITIONAL INFORMATION

  • Has associated issue: Fixes When grouping in a pivot table, part of the bottom border is not displayed. #36031
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

@bito-code-review
Copy link
Contributor

bito-code-review bot commented Nov 12, 2025 •
edited
Loading

Code Review Agent Run #53dce2

Actionable Suggestions - 0
Review Details
  • Files reviewed - 2 · Commit Range: 5da7d60..5da7d60
    • superset-frontend/plugins/plugin-chart-pivot-table/src/react-pivottable/Styles.js
    • superset-frontend/plugins/plugin-chart-pivot-table/src/react-pivottable/TableRenderers.jsx
  • Files skipped - 0
  • Tools
    • Eslint (Linter) - ✔︎ Successful
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful
Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers a full AI review.

  • /pause - Pauses automatic reviews on this pull request.

  • /resume - Resumes automatic reviews.

  • /resolve - Marks all Bito-posted review comments as resolved.

  • /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Default Agent You can customize the agent settings here or contact your Bito workspace admin at [email protected].

Documentation & Help

AI Code Review powered by Bito Logo

@dosubot dosubot bot added the viz:charts:pivot Related to the Pivot Table charts label Nov 12, 2025
korbit-ai[bot]
korbit-ai bot suggested changes Nov 12, 2025
View reviewed changes
Copy link

@korbit-ai korbit-ai bot left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Review by Korbit AI

Korbit automatically attempts to detect when you fix issues in new commits.
Category Issue Status
Performance Redundant calculation in row rendering loop ▹ view
Files scanned ​
File Path Reviewed
superset-frontend/plugins/plugin-chart-pivot-table/src/react-pivottable/Styles.js ✅
superset-frontend/plugins/plugin-chart-pivot-table/src/react-pivottable/TableRenderers.jsx ✅

Explore our documentation to understand the languages and file types we support and the files we ignore.

Check out our docs on how you can make Korbit work best for you and your team.

Loving Korbit!? Share us on LinkedIn Reddit and X

Copilot finished reviewing on behalf of sadpandajoe November 13, 2025 18:25
Copilot AI reviewed Nov 13, 2025
View reviewed changes
Copy link
Contributor

Copilot AI left a 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 fixes a missing bottom border on the last row label in Pivot Table visualizations when grouping is expanded. The fix adds logic to detect cells that span to the last data row and applies a pvtRowLabelLast CSS class with appropriate border-bottom styling.

  • Adds totalRowsCount calculation to track the number of visible rows
  • Applies pvtRowLabelLast class to row header cells that span to the last row
  • Adds CSS styling for the bottom border on last row labels

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
TableRenderers.jsx Adds logic to calculate total row count and conditionally apply pvtRowLabelLast class to cells spanning to the last row
Styles.js Adds CSS rule for bottom border on last row label cells

superset-frontend/plugins/plugin-chart-pivot-table/src/react-pivottable/TableRenderers.jsx Outdated
maxColVisible: Math.max(...visibleColKeys.map(k => k.length)),
rowAttrSpans: this.calcAttrSpans(visibleRowKeys, rowAttrs.length),
colAttrSpans: this.calcAttrSpans(visibleColKeys, colAttrs.length),
totalRowsCount: visibleRowKeys.length + (colTotals ? 1 : 0),
Copy link

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

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

The totalRowsCount calculation incorrectly includes the column totals row. The totals row is rendered separately via renderTotalsRow() and should not be counted when determining if a data row cell is the last row.

When colTotals is enabled, a cell in the last data row with rowSpan=1 will have rowIdx + rowSpan = N (where N is visibleRowKeys.length), but this will never equal totalRowsCount = N + 1, so the border will never be applied.

The calculation should be:

totalRowsCount: visibleRowKeys.length,
Suggested change
totalRowsCount: visibleRowKeys.length + (colTotals ? 1 : 0),
totalRowsCount: visibleRowKeys.length,

Copilot uses AI. Check for mistakes.
@sadpandajoe
Copy link
Member

sadpandajoe commented Nov 13, 2025

@igor-tech can we add some tests here?

@pull-request-size pull-request-size bot added size/L and removed size/S labels Nov 14, 2025
@rusackas
Copy link
Member

rusackas commented Nov 17, 2025

Approving CI workflows, but deferring to @sadpandajoe otherwise ;)

@rusackas rusackas requested a review from SBIN2010 November 17, 2025 22:48
@SBIN2010 SBIN2010 added the 🎪 ⚡ showtime-trigger-start Create new ephemeral environment for this PR label Nov 17, 2025
@github-actions github-actions bot added 🎪 f6af157 🚦 building Environment f6af157 status: building 🎪 f6af157 📅 2025-11-17T22-55 Environment f6af157 created at 2025-11-17T22-55 🎪 f6af157 ⌛ 48h Environment f6af157 expires after 48h 🎪 f6af157 🤡 SBIN2010 Environment f6af157 requested by SBIN2010 and removed 🎪 ⚡ showtime-trigger-start Create new ephemeral environment for this PR labels Nov 17, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Nov 17, 2025

🎪 Showtime is building environment on GHA for f6af157

@github-actions github-actions bot added 🎪 f6af157 🚦 deploying Environment f6af157 status: deploying 🎪 f6af157 🚦 running Environment f6af157 status: running 🎪 🎯 f6af157 Active environment pointer - f6af157 is receiving traffic 🎪 f6af157 🌐 35.91.119.142:8080 Environment f6af157 URL: http://35.91.119.142:8080 (click to visit) and removed 🎪 f6af157 🚦 building Environment f6af157 status: building 🎪 f6af157 🚦 deploying Environment f6af157 status: deploying 🎪 f6af157 🚦 running Environment f6af157 status: running 🎪 🎯 f6af157 Active environment pointer - f6af157 is receiving traffic labels Nov 17, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Nov 17, 2025

🎪 Showtime deployed environment on GHA for f6af157

• Environment: http://35.91.119.142:8080 (admin/admin)
• Lifetime: 48h auto-cleanup
• Updates: New commits create fresh environments automatically

@SBIN2010
Copy link
Contributor

SBIN2010 commented Nov 17, 2025

Works correctly. Thank you.
@sadpandajoe @rusackas What are your thoughts on the tests?

@github-actions github-actions bot removed 🎪 f6af157 🤡 SBIN2010 Environment f6af157 requested by SBIN2010 🎪 f6af157 🚦 running Environment f6af157 status: running 🎪 f6af157 📅 2025-11-17T22-55 Environment f6af157 created at 2025-11-17T22-55 🎪 f6af157 🌐 35.91.119.142:8080 Environment f6af157 URL: http://35.91.119.142:8080 (click to visit) 🎪 f6af157 ⌛ 48h Environment f6af157 expires after 48h labels Nov 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@sadpandajoe sadpandajoe Awaiting requested review from sadpandajoe

@SBIN2010 SBIN2010 Awaiting requested review from SBIN2010

+1 more reviewer

@korbit-ai korbit-ai[bot] korbit-ai[bot] requested changes

Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

dependencies:npm plugins size/L viz:charts:pivot Related to the Pivot Table charts

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

When grouping in a pivot table, part of the bottom border is not displayed.

4 participants

@igor-tech @sadpandajoe @rusackas @SBIN2010