Skip to content

fix(grid): Make sure border does not appear when new theme is loaded runtime.#17030

Open
MayaKirova wants to merge 4 commits into21.1.xfrom
mkirova/fix-3903-21.1.x
Open

fix(grid): Make sure border does not appear when new theme is loaded runtime.#17030
MayaKirova wants to merge 4 commits into21.1.xfrom
mkirova/fix-3903-21.1.x

Conversation

@MayaKirova
Copy link
Copy Markdown
Contributor

@MayaKirova MayaKirova commented Mar 11, 2026

Closes IgniteUI/igniteui-angular-samples#3903

  • Understand current theme.css approach
  • Create alternate theme SCSS using igniteui-angular dark material preset
  • Add alternate theme to angular.json as non-injected style bundle (inject: false)
  • Remove theme.css asset copy from angular.json assets
  • Update grid-cellMerging component to reference bundled stylesheet (grid-cell-merging-alternate-theme.css)
  • Implement OnDestroy cleanup to remove link element on navigation
  • Delete the large theme.css file (15179 lines)
  • Fix border reset in %igx-grid__tr--merged to use both border-bottom: 0 and border-block-end: 0 (removes !important)

@MayaKirova MayaKirova changed the title fix(grid): Exclude border from rowHeight calc. fix(grid): Make sure border does not appear when new theme is loaded runtime. Mar 11, 2026
@dkamburov dkamburov requested a review from skrustev April 2, 2026 15:52
@dkamburov dkamburov added ✅ status: verified Applies to PRs that have passed manual verification and removed ❌ status: awaiting-test PRs awaiting manual verification labels Apr 2, 2026
Copilot AI review requested due to automatic review settings April 3, 2026 06:57
Copy link
Copy Markdown
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

Fixes a grid cell-merging visual regression where a row border can appear after loading a new theme stylesheet at runtime, and adds a sample-level runtime theme toggle to reproduce/verify the scenario.

Changes:

  • Added a runtime theme CSS toggle in the grid-cellMerging sample (signals + DOM <link> injection).
  • Updated grid theme styling for merged rows to ensure the merged-row bottom border is removed when themes are applied dynamically.
  • Removed a private-flag-based virtualization size recalculation from IgxRowDirective.getRowHeight().

Reviewed changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/app/grid-cellMerging/grid-cellMerging.component.ts Adds toggleTheme() and signal state for runtime loading/unloading of a theme stylesheet.
src/app/grid-cellMerging/grid-cellMerging.component.html Adds a UI button to trigger the runtime theme toggle.
projects/igniteui-angular/grids/core/src/row.directive.ts Removes _cdrRequests-driven recalcUpdateSizes() side effect from row height calculation.
projects/igniteui-angular/core/src/core/styles/components/grid/_grid-theme.scss Adjusts merged-row border reset to prevent borders appearing after runtime theme load.
angular.json Copies src/app/grid-cellMerging/theme.css into build assets so it can be loaded at runtime.

@ChronosSF
Copy link
Copy Markdown
Member

@MayaKirova , is it needed to add the huge theme.css to the samples project? I think you should be able to add the default themes through angular.json to the build and switch them runtime this way?

@MayaKirova
Copy link
Copy Markdown
Contributor Author

@MayaKirova , is it needed to add the huge theme.css to the samples project? I think you should be able to add the default themes through angular.json to the build and switch them runtime this way?

@ChronosSF
I was trying to make it as similar as possible to the original issue, where new theme css is runtime injected via the theming widget over the existing theme. Mainly for testing.

Not sure if the scenario where the default theme is switched is the same, but sure. @copilot add the default themes through angular.json to the build and switch them runtime this way in the grid-cellMerging sample.

…e theme; add OnDestroy cleanup

Agent-Logs-Url: https://github.com/IgniteUI/igniteui-angular/sessions/12bdcdf6-0c85-490e-b44e-497f597e2474

Co-authored-by: MayaKirova <10397980+MayaKirova@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 3, 2026

@MayaKirova , is it needed to add the huge theme.css to the samples project? I think you should be able to add the default themes through angular.json to the build and switch them runtime this way?
...

Done in commit 061341a. The changes:

  1. Removed theme.css – the 15,000-line pre-compiled CSS file is gone.
  2. Added src/styles/grid-cell-merging-alternate-theme.scss – a minimal SCSS file that uses the dark material schema from the library itself, compiled by the Angular build.
  3. angular.json – added the new SCSS as a non-injected style bundle (inject: false, bundleName: "grid-cell-merging-alternate-theme"). This produces grid-cell-merging-alternate-theme.css in the build output without auto-injecting it into index.html.
  4. ComponenttoggleTheme() now loads grid-cell-merging-alternate-theme.css (the bundled file) at runtime. Also implemented ngOnDestroy to clean up the <link> element when navigating away.

@MayaKirova MayaKirova added ❌ status: awaiting-test PRs awaiting manual verification and removed ✅ status: verified Applies to PRs that have passed manual verification labels Apr 3, 2026
@MayaKirova
Copy link
Copy Markdown
Contributor Author

@dkamburov resetting verified, since co-pilot changed the fix. Moving back to awaiting test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants