fix(pivot-grid): match scroll-start border/background to header cells - #17599
Open
simeonoff wants to merge 3 commits into
Open
fix(pivot-grid): match scroll-start border/background to header cells#17599simeonoff wants to merge 3 commits into
simeonoff wants to merge 3 commits into
Conversation
simeonoff
added
❌ status: awaiting-test
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Add the requested regression coverage and use declarative host metadata for the static class.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Updates Pivot Grid styling so scroll-start borders/backgrounds match header cells and removes an inherited filler border.
Changes:
- Centralizes shared grid Sass variables.
- Adds scoped Pivot Grid styling and host stylesheet wiring.
- Removes the filler wrapper’s separator border.
File summaries
| File | Review notes |
|---|---|
projects/igniteui-angular/grids/themes/_variables.scss |
Shared Sass variables extracted. |
projects/igniteui-angular/grids/themes/_base.scss |
Moderate: add regression coverage for the filler wrapper border removal. |
projects/igniteui-angular/grids/pivot-grid/src/themes/grid/_base.scss |
Moderate: add regression coverage for scoped scroll-start styling. |
projects/igniteui-angular/grids/pivot-grid/src/pivot-grid.component.ts |
Nit: declare the static host class in component host metadata. |
projects/igniteui-angular/grids/pivot-grid/src/pivot-grid.component.scss |
Loads the Pivot Grid theme styles. |
Review details
Suppressed comments (1)
projects/igniteui-angular/grids/themes/_base.scss:2764
- This new row-filler border rule is also untested. Please extend the pivot-grid regression coverage to render the empty bottom filler and verify that its reused
igx-grid-thead__wrapperhas no separator border; otherwise this part of the visual fix can regress independently of the scroll-start assertions.
border-bottom: none;
- Files reviewed: 5/5 changed files
- Comments generated: 2
- Review effort level: Lite
Co-authored-by: simeonoff <4118303+simeonoff@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #17584
Description
IgxPivotGridComponentno longer inherits the shared, unscoped.igx-grid__scroll-start--pinnedbold border/content-backgroundfill used byIgxGrid/IgxTreeGrid/IgxHierarchicalGrid. A new.igx-grid--pivothost class scopes pivot-only overrides (border matches the plain header border, background matchesheader-background) via a dedicatedpivot-grid.component.scss/themes/grid/_base.scsspair, wired through Angular's normalstyleUrlinstead of touching the shared grid theme file.Also removes the stray
border-bottominherited by.igx-pivot-grid-row-filler__wrapper's reused header markup. Shared grid Sass variables ($grid-header-border,$grid-pinned-border, z-indices, etc.) were extracted intogrids/themes/_variables.scssso both the shared grid theme and the new pivot-grid theme file can consume them without duplication.Motivation / Context
Fixes #17584 — the pivot grid rendered a bold "pinned column" border (and mismatched background) on the horizontal scrollbar's leading edge, even though pivot's row dimensions aren't part of the public column-pinning API. The border should visually match the plain header/cell border pivot's row dimension cells actually use.
Type of Change (check all that apply):
Component(s) / Area(s) Affected:
Pivot Grid (theming/styles only —
grids/pivot-grid,grids/themes)How Has This Been Tested?
Verified
npm run build:styles:componentscompiles the new/changed SCSS without errors and produces the expected scoped selectors (.igx-grid--pivot .igx-grid__scroll-start(--pinned),.igx-pivot-grid-row-filler__wrapper .igx-grid-thead__wrapper), confirmednpm run lint:stylesandeslintpass, and confirmed the unrelatedgrid-base.styles.tsoutput (flat/tree/hierarchical grids) is unchanged after the variable extraction. Visual/browser verification against the pivot hierarchy dev sample and the pivot-grid Karma suite (npm run test:lib:pgrid) are still outstanding.Checklist:
feature/README.MDupdates for the feature docsREADME.MDCHANGELOG.MDupdates for newly added functionalityng updatemigrations for the breaking changes (migrations guidelines)