Skip to content

Support group header hover and regular bg #927

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

Merged
merged 7 commits into from
Jun 18, 2025

Conversation

rferreira98
Copy link
Contributor

@rferreira98 rferreira98 commented Mar 20, 2024

@jassmith

With this PR we'll be able to customize background colors for group headers and also change the hover color as well without affecting regular headers.

@hakobyanheghine
Copy link

Hey @jassmith ,

When can we get this merged? This would be very helpful for my case as well!

Thanks!

@sergeynersesyan
Copy link

This is very helpful feature. Waiting for it to be merged

@lukasmasuch lukasmasuch requested a review from Copilot June 16, 2025 22:50
Copy link
Contributor

@Copilot 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 adds support for custom background and hover colors specifically for group headers without affecting regular headers.

  • Introduce two new theme properties: bgGroupHeader and bgGroupHeaderHovered
  • Update the drawing logic to use these new properties with proper fallbacks
  • Expose corresponding CSS variables and document the bgGroupHeader variable in the theming guide

Reviewed Changes

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

File Description
packages/core/src/internal/data-grid/render/data-grid-render.header.ts Use bgGroupHeader/bgGroupHeaderHovered in fillColor with fallback to bgHeader
packages/core/src/docs/08-theming.stories.tsx Added a docs entry for the bgGroupHeader CSS variable
packages/core/src/common/styles.ts Added CSS variables and theme interface fields for group header backgrounds
Comments suppressed due to low confidence (2)

packages/core/src/docs/08-theming.stories.tsx:257

  • The theming docs lack an entry for the new bgGroupHeaderHovered CSS variable (--gdg-bg-group-header-hovered). Please add a row describing its purpose and default fallback behavior.
| bgGroupHeader | --gdg-bg-group-header | string \\| undefined | The group header background color, if none provided the `bgHeader` is used instead. |

packages/core/src/internal/data-grid/render/data-grid-render.header.ts:185

  • New behavior for group header backgrounds and hover styling is introduced here. Please add or update unit/integration tests to verify that bgGroupHeader and bgGroupHeaderHovered are applied correctly under various override scenarios.
export function drawGroups(

Copy link
Collaborator

@lukasmasuch lukasmasuch left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@lukasmasuch lukasmasuch merged commit d1b79d3 into glideapps:main Jun 18, 2025
4 checks passed
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.

4 participants