feat(components/modals): add modal banner (#4275)#4280
Merged
Blackbaud-PaulCrowder merged 2 commits intomainfrom Mar 12, 2026
Merged
Conversation
blackbaud-sky-build-user
added
the
risk level (author): 2
blackbaud-sky-build-user
temporarily deployed
to
e2e-team-members
GitHub Actions
Inactive
|
View your CI Pipeline Execution ↗ for commit 0ecdb4e
☁️ Nx Cloud last updated this comment at |
Contributor
📝 WalkthroughWalkthroughThis PR introduces a new Changes
Sequence DiagramsequenceDiagram
participant User
participant ModalVisualComponent
participant SkyModalService
participant SkyModalComponent
participant SkyModalBannerComponent
participant DOM
User->>ModalVisualComponent: Click "Open Banner Modal"
ModalVisualComponent->>SkyModalService: openModal(ModalComponent, {providers: [...]})
SkyModalService->>SkyModalComponent: Create modal instance with context
SkyModalComponent->>DOM: Render modal with headingHidden binding
SkyModalComponent->>SkyModalBannerComponent: Project banner content (ng-content)
SkyModalBannerComponent->>DOM: Render banner with backgroundImage style
SkyModalBannerComponent->>DOM: Apply imageSrc to background-image CSS
DOM->>User: Display modal with banner and content
User->>SkyModalComponent: Click close button
SkyModalComponent->>SkyModalService: Close modal instance
SkyModalService->>DOM: Remove modal from DOM
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
libs/components/modals/documentation.json (1)
59-65:⚠️ Potential issue | 🟠 MajorAdd the new public banner component to the modal API docs.
ModalsModalBasicWithBannerExampleComponentis registered here, butSkyModalBannerComponentis now part of the public modals API and is still missing fromgroups.modal.development.docsIds. That leaves the feature discoverable through the example, but not through the API reference.As per coding guidelines, `**/libs/components/*/documentation.json`: "Add class names to the component's `documentation.json` file under `codeExamples` section and keep examples in sync with component and harness changes".📝 Suggested docs update
"development": { "docsIds": [ "SkyModalModule", "SkyModalComponent", + "SkyModalBannerComponent", "SkyModalHeaderComponent", "SkyModalContentComponent",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/components/modals/documentation.json` around lines 59 - 65, Update the modal API docs to include the new public banner component by adding "SkyModalBannerComponent" to the documentation JSON array used for API grouping; specifically, add "SkyModalBannerComponent" to the groups.modal.development.docsIds (alongside the existing "ModalsModalBasicWithBannerExampleComponent") in the component's documentation.json so the SkyModalBannerComponent appears in the API reference.
🧹 Nitpick comments (10)
apps/playground/src/app/components/modal/modal-banner-image.component.ts (1)
4-7: SetOnPushon this new component.This component is stateless, so it should opt into the repo's default change-detection strategy.
Suggested change
-import { Component, inject } from '@angular/core'; +import { ChangeDetectionStrategy, Component, inject } from '@angular/core'; @@ `@Component`({ + changeDetection: ChangeDetectionStrategy.OnPush, imports: [SkyModalModule], templateUrl: './modal-banner-image.component.html', })As per coding guidelines "Set
changeDetection: ChangeDetectionStrategy.OnPushin@Componentdecorator".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/playground/src/app/components/modal/modal-banner-image.component.ts` around lines 4 - 7, The component decorator for ModalBannerImageComponent should opt into OnPush change detection: import ChangeDetectionStrategy from '@angular/core' and add changeDetection: ChangeDetectionStrategy.OnPush to the `@Component` metadata (the decorator that currently contains imports and templateUrl) so the stateless component uses the repo default strategy.apps/playground/src/app/components/modal/modal-banner-content.component.scss (1)
1-11: Add the compat mixin scaffold here too.This new SCSS file is also missing the required
compatMixinsimport and top-of-file override mixins.As per coding guidelines "Add compat mixins to SCSS component files: Ensure the import
@use 'libs/components/theme/src/lib/styles/compat-tokens-mixins' as compatMixins;is included" and "Include the mixincompatMixins.sky-default-overridesoutside of any selectors".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/playground/src/app/components/modal/modal-banner-content.component.scss` around lines 1 - 11, This SCSS file is missing the compat mixins import and top-level override mixin; add the import statement for the compat mixins module using the namespace compatMixins (i.e. `@use` 'libs/components/theme/src/lib/styles/compat-tokens-mixins' as compatMixins) at the top of modal-banner-content.component.scss and invoke the top-level mixin compatMixins.sky-default-overrides outside any selector blocks (before .banner-content) so the overrides apply globally to this component.apps/playground/src/app/components/modal/modal-banner-content.component.ts (1)
4-8: SetOnPushon this component as well.This one is also stateless and should follow the standard component change-detection setting.
Suggested change
-import { Component, inject } from '@angular/core'; +import { ChangeDetectionStrategy, Component, inject } from '@angular/core'; @@ `@Component`({ + changeDetection: ChangeDetectionStrategy.OnPush, imports: [SkyModalModule], templateUrl: './modal-banner-content.component.html', styleUrl: './modal-banner-content.component.scss', })As per coding guidelines "Set
changeDetection: ChangeDetectionStrategy.OnPushin@Componentdecorator".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/playground/src/app/components/modal/modal-banner-content.component.ts` around lines 4 - 8, The component decorator for ModalBannerContentComponent lacks the OnPush change-detection setting; update the `@Component` on ModalBannerContentComponent to include changeDetection: ChangeDetectionStrategy.OnPush and add the corresponding import for ChangeDetectionStrategy from '@angular/core' so the component uses OnPush semantics. Ensure you update the imports section to include ChangeDetectionStrategy and leave the rest of the decorator intact.libs/components/modals/src/lib/modules/modal/modal-content.component.scss (1)
21-25: Expose this new spacing through an override token.This hard-codes component tokens in a new rule, so consumers can't tune the banner/content spacing through
sky-default-overrides. Please add an override variable to the existing mixin and use the currentcalc(...)as the fallback here.As per coding guidelines "When using a CSS variable with its current value, find the existing location matching the variable name, assign the new variable that value, and use the new variable in the current location with the given fallback".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/components/modals/src/lib/modules/modal/modal-content.component.scss` around lines 21 - 25, Add an override CSS variable for the banner/content spacing and use it as the padding-top fallback: inside the existing mixin that defines component tokens, create a new variable (e.g. --sky-comp-modal-banner-content-space-inset) and initialize it to the current calc(...) expression using the existing tokens var(--sky-comp-modal-header-space-inset-bottom) and var(--sky-comp-modal-content-space-inset-top); then replace the hard-coded padding-top in the sky-modal-banner + sky-modal-content rule with padding-top: var(--sky-comp-modal-banner-content-space-inset, calc(...)). Ensure the new variable is exposed so consumers can override via sky-default-overrides and reference the same tokens used originally.libs/components/modals/src/lib/modules/modal/modal-banner.component.scss (1)
1-9: Add the required compat mixin scaffold to this new stylesheet.This component SCSS skips the repo's compat-token import and top-of-file override mixins, so it won't participate in the standard override path. Please add the required
compatMixinsscaffold before the:hostselector.As per coding guidelines "Add compat mixins to SCSS component files: Ensure the import
@use 'libs/components/theme/src/lib/styles/compat-tokens-mixins' as compatMixins;is included" and "Create empty compat mixins ... positioned at the top of the SCSS file directly below imports and before any other selectors".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/components/modals/src/lib/modules/modal/modal-banner.component.scss` around lines 1 - 9, Add the required compat mixins scaffold at the top of modal-banner.component.scss by adding the import `@use` 'libs/components/theme/src/lib/styles/compat-tokens-mixins' as compatMixins; and then invoking/declaring the empty compat mixin hooks (e.g., compatMixins.override and compatMixins.override-top or the repo-standard mixin names) immediately below the import and before the existing :host selector so the component participates in the standard override path.apps/e2e/modals-storybook/src/app/modal/modal.component.ts (1)
95-187: Consider extracting a shared context-opening helper.Lines 95-187 repeat the same
providers/useFactoryscaffolding five times and only vary a fewModalTestContextfields. Pulling that into one helper would reduce copy/paste drift as more modal variants get added.♻️ Possible simplification
+ private openContextModal(overrides: Partial<ModalTestContext>): void { + this.openModal(ModalBasicComponent, { + providers: [ + { + provide: ModalTestContext, + useFactory: (): ModalTestContext => + Object.assign(new ModalTestContext(), overrides), + }, + ], + }); + } + protected onOpenHeadingHiddenModalClick(): void { - this.openModal(ModalBasicComponent, { - providers: [ - { - provide: ModalTestContext, - useFactory: (): ModalTestContext => { - const context = new ModalTestContext(); - context.headingHidden = true; - context.headingText = 'My heading'; - context.modalContent = 'Modal content'; - - return context; - }, - }, - ], + this.openContextModal({ + headingHidden: true, + headingText: 'My heading', + modalContent: 'Modal content', }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/e2e/modals-storybook/src/app/modal/modal.component.ts` around lines 95 - 187, Extract the repeated provider/useFactory block into a single helper (e.g., createModalContextProvider or buildModalContext) that accepts partial ModalTestContext values and returns the providers array used by openModal; replace each of the methods (onOpenHeadingHiddenModalClick, onOpenBannerContentModalClick, onOpenBannerImageModalClick, onOpenBannerContentImageModalClick, onOpenBannerHeadingVisibleModalClick) to call openModal(ModalBasicComponent, { providers: createModalContextProvider({ ... }) }) and pass only the differing fields (bannerContent, bannerImageSrc, headingHidden, headingText, modalContent) so the scaffolding is centralized and duplication removed while preserving behavior.apps/e2e/modals-storybook/src/app/modal/modal.component.html (1)
67-116: Use class bindings instead of[ngClass]on these new buttons.These additions only toggle one class, so
[class.modal-screenshot-buttons-hidden]="buttonsHidden"is the simpler match for the repo template rule.♻️ Suggested template change
- [ngClass]="{ - 'modal-screenshot-buttons-hidden': buttonsHidden - }" + [class.modal-screenshot-buttons-hidden]="buttonsHidden"As per coding guidelines, "Do NOT use
ngClass; useclassbindings instead in Angular templates".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/e2e/modals-storybook/src/app/modal/modal.component.html` around lines 67 - 116, These buttons use [ngClass] to toggle a single class; replace each [ngClass]="{ 'modal-screenshot-buttons-hidden': buttonsHidden }" with a class binding [class.modal-screenshot-buttons-hidden]="buttonsHidden" on the buttons that call onOpenHeadingHiddenModalClick, onOpenBannerContentModalClick, onOpenBannerImageModalClick, onOpenBannerContentImageModalClick, and onOpenBannerHeadingVisibleModalClick so they follow the repo template (use class bindings instead of ngClass).apps/playground/src/app/components/modal/modal-visual.component.html (1)
145-158: Give the new demo buttons uniquesky-test-*hooks.These two launchers reuse
.sky-test-modal-required-field, which is already attached to several other modal buttons above. Any automation keyed off that class now has no deterministic way to target the banner variants.♻️ Suggested update
- class="sky-btn sky-btn-primary sky-test-modal-required-field" + class="sky-btn sky-btn-primary sky-test-modal-banner-image" ... - class="sky-btn sky-btn-primary sky-test-modal-required-field" + class="sky-btn sky-btn-primary sky-test-modal-banner-content"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/playground/src/app/components/modal/modal-visual.component.html` around lines 145 - 158, The two buttons that open banner modals reuse the generic test hook class and should get unique hooks: update the button element for openBannerImageModal() to use a distinct test hook (e.g., replace or add a class like "sky-test-modal-banner-image") and update the button for openBannerContentModal() to use a different distinct test hook (e.g., "sky-test-modal-banner-content"); ensure you change the class attribute on the button elements that call openBannerImageModal() and openBannerContentModal() so automation can target each launcher deterministically.apps/playground/src/app/components/modal/modal-banner-content.component.html (1)
1-5: Align the hidden modal title with the visible banner heading.With
headingHiddenenabled, assistive tech still gets the modal title fromheadingText, but sighted users seeBanner contentin the banner. Keeping those labels in sync makes the demo less confusing.♿ Suggested update
-<sky-modal headingText="Test" headingHidden> +<sky-modal headingText="Banner content" headingHidden>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/playground/src/app/components/modal/modal-banner-content.component.html` around lines 1 - 5, The visible banner heading ("Banner content" inside <sky-modal-banner> .banner-content h2) is out of sync with the modal's hidden accessible title (sky-modal headingText used with headingHidden), so either set the modal's headingText to the same string or wire accessibility to the visible heading: add a unique id to the h2 (e.g., banner-heading) and set the modal to reference it via aria-labelledby on <sky-modal> (or update headingText to match the h2). Ensure the chosen approach keeps <sky-modal headingText/headingHidden> and the banner h2 content identical for sighted and assistive users.libs/components/code-examples/src/lib/modules/modals/modal/with-banner/example.component.ts (1)
1-18: Align the example metadata with the new banner variant.Line 10 still uses the
with-controllerselector, and this component is also missingOnPush. Both look copied from the controller example.As per coding guidelines, `Set changeDetection: ChangeDetectionStrategy.OnPush` in `@Component` decorator.♻️ Suggested cleanup
-import { Component, OnDestroy, inject } from '@angular/core'; +import { + ChangeDetectionStrategy, + Component, + OnDestroy, + inject, +} from '@angular/core'; @@ `@Component`({ - selector: 'app-modals-modal-basic-with-controller-example', + selector: 'app-modals-modal-basic-with-banner-example', + changeDetection: ChangeDetectionStrategy.OnPush, template: `<button🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/components/code-examples/src/lib/modules/modals/modal/with-banner/example.component.ts` around lines 1 - 18, Update the `@Component` metadata to use the "with-banner" variant selector and enable OnPush change detection: change the selector string from 'app-modals-modal-basic-with-controller-example' to 'app-modals-modal-basic-with-banner-example', import ChangeDetectionStrategy from '@angular/core', and add changeDetection: ChangeDetectionStrategy.OnPush to the `@Component` decorator; ensure the component class that currently implements OnDestroy remains unchanged (no runtime behavior changes required).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/e2e/modals-storybook-e2e/src/e2e/modal.component.cy.ts`:
- Around line 21-25: The shared modal variant list includes 'heading-hidden' but
the shared test flow assumes the header close button is visible and runs
cy.get('.sky-btn-close').should('be.visible').click(), which fails for the
headingHidden() case; remove 'heading-hidden' from the shared matrix and add a
separate test path for the headingHidden() scenario that does not expect or
click the close button (instead assert the header close is hidden or skip that
step), updating references where the shared matrix is iterated and the test that
calls cy.get('.sky-btn-close').should('be.visible').click() to exclude the
headingHidden() case.
In
`@libs/components/code-examples/src/lib/modules/modals/modal/with-banner/modal.component.ts`:
- Around line 1-35: The component is missing ChangeDetectionStrategy.OnPush;
import ChangeDetectionStrategy from '@angular/core' and add changeDetection:
ChangeDetectionStrategy.OnPush to the `@Component` decorator in modal.component.ts
(alongside the existing imports/metadata) so the component uses OnPush change
detection.
In `@libs/components/modals/src/lib/modules/modal/modal-banner.component.ts`:
- Around line 1-16: The component metadata for SkyModalBannerComponent must opt
into OnPush change detection; add an import for ChangeDetectionStrategy from
'@angular/core' and set changeDetection: ChangeDetectionStrategy.OnPush in the
`@Component` decorator (next to selector/host/templateUrl/etc.) so the
signal-backed presentational component uses OnPush change detection; update the
`@Component` metadata for SkyModalBannerComponent accordingly and keep references
to the existing host bindings (e.g., backgroundImage).
In `@libs/components/modals/src/lib/modules/modal/modal.component.html`:
- Around line 23-24: The modal header currently sets aria-labelledby to
headerId.id while that element can be hidden by headingHidden(), risking an
unnamed dialog; update the template so aria-labelledby is only set when the
header is visible (e.g., bind attr.aria-labelledby to headerId.id when
headingHidden() is false) and provide an alternate accessible label when
headingHidden() is true (e.g., set attr.aria-label to dialogLabel or another
explicit label). Also replace usage of [ngStyle] and [ngClass] in this file (and
the noted occurrences) with native Angular style and class bindings (use
[style.someProp] or [style]="..." and [class.someClass] or class="...") so you
remove [ngStyle] and [ngClass] usages while keeping the same styling and
conditional class logic; reference the header element (headerId.id,
headingHidden(), dialogLabel) and each template binding location to update.
In `@libs/components/modals/src/lib/modules/modal/modal.component.spec.ts`:
- Around line 199-220: The tests using openModalWithHeadingContext() are not
exercising the headingText-only rendering path because ModalTestComponent always
projects a <sky-modal-header> slot; add a new test (or modify the helper) to
open a component that does not project any <sky-modal-header> so the modal must
render using ModalTestContext.headingText instead of projected content.
Specifically, create or reuse a lightweight test component (e.g.,
ModalHeadingOnlyTestComponent) that does not include <sky-modal-header>, call
openModal with that component using openModalWithHeadingContext(headingHidden,
async) or a similar helper, and assert the headingText-only DOM path
renders/does not render when headingHidden toggles; keep references to
ModalTestContext, openModalWithHeadingContext, and ModalTestComponent to locate
and update the tests.
In
`@libs/components/modals/testing/src/modules/modal/modal-banner-harness.spec.ts`:
- Around line 137-177: Add a predicate-based test that verifies the public
filter API by using SkyModalBannerHarness.with(...) (e.g., with dataSkyId) when
obtaining the banner from the modal harness: open a modal that includes a banner
with a known dataSkyId, call loader.getHarness(SkyModalHarness) then
modalHarness.getBanner(SkyModalBannerHarness.with({ dataSkyId: 'expected-id' }))
and assert it resolves to a SkyModalBannerHarness instance; also add a negative
case that queries with a non-matching dataSkyId and asserts getBanner(...)
resolves to null. Ensure the tests reference SkyModalBannerHarness.with,
modalHarness.getBanner, and bannerHarness.getImageSrc where appropriate.
- Around line 100-113: The TestBed configuration for the modal harness spec is
missing the TestButtonComponent declaration causing
TestBed.createComponent(TestButtonComponent) to fail; update the
TestBed.configureTestingModule call to include TestButtonComponent in the
declarations array alongside BannerModalComponent, NoBannerModalComponent, and
NonMatchingBannerModalComponent so the test fixture can be created and the
harness can run.
In `@libs/components/modals/testing/src/modules/modal/modal-banner-harness.ts`:
- Around line 6-34: Add a standard static with() entry point and a
SkyModalBannerHarnessFilters interface to the SkyModalBannerHarness so callers
can use predicate-based filtering (e.g., dataSkyId). Define export interface
SkyModalBannerHarnessFilters with optional properties used by other harness
filters (at minimum dataSkyId?: string), and implement public static
with(filters?: SkyModalBannerHarnessFilters) that returns a
HarnessPredicate<SkyModalBannerHarness> configured to filter by the provided
dataSkyId (use HarnessPredicate.stringIncludes and the hostSelector), mirroring
the pattern used by other SKY harnesses.
---
Outside diff comments:
In `@libs/components/modals/documentation.json`:
- Around line 59-65: Update the modal API docs to include the new public banner
component by adding "SkyModalBannerComponent" to the documentation JSON array
used for API grouping; specifically, add "SkyModalBannerComponent" to the
groups.modal.development.docsIds (alongside the existing
"ModalsModalBasicWithBannerExampleComponent") in the component's
documentation.json so the SkyModalBannerComponent appears in the API reference.
---
Nitpick comments:
In `@apps/e2e/modals-storybook/src/app/modal/modal.component.html`:
- Around line 67-116: These buttons use [ngClass] to toggle a single class;
replace each [ngClass]="{ 'modal-screenshot-buttons-hidden': buttonsHidden }"
with a class binding [class.modal-screenshot-buttons-hidden]="buttonsHidden" on
the buttons that call onOpenHeadingHiddenModalClick,
onOpenBannerContentModalClick, onOpenBannerImageModalClick,
onOpenBannerContentImageModalClick, and onOpenBannerHeadingVisibleModalClick so
they follow the repo template (use class bindings instead of ngClass).
In `@apps/e2e/modals-storybook/src/app/modal/modal.component.ts`:
- Around line 95-187: Extract the repeated provider/useFactory block into a
single helper (e.g., createModalContextProvider or buildModalContext) that
accepts partial ModalTestContext values and returns the providers array used by
openModal; replace each of the methods (onOpenHeadingHiddenModalClick,
onOpenBannerContentModalClick, onOpenBannerImageModalClick,
onOpenBannerContentImageModalClick, onOpenBannerHeadingVisibleModalClick) to
call openModal(ModalBasicComponent, { providers: createModalContextProvider({
... }) }) and pass only the differing fields (bannerContent, bannerImageSrc,
headingHidden, headingText, modalContent) so the scaffolding is centralized and
duplication removed while preserving behavior.
In
`@apps/playground/src/app/components/modal/modal-banner-content.component.html`:
- Around line 1-5: The visible banner heading ("Banner content" inside
<sky-modal-banner> .banner-content h2) is out of sync with the modal's hidden
accessible title (sky-modal headingText used with headingHidden), so either set
the modal's headingText to the same string or wire accessibility to the visible
heading: add a unique id to the h2 (e.g., banner-heading) and set the modal to
reference it via aria-labelledby on <sky-modal> (or update headingText to match
the h2). Ensure the chosen approach keeps <sky-modal headingText/headingHidden>
and the banner h2 content identical for sighted and assistive users.
In
`@apps/playground/src/app/components/modal/modal-banner-content.component.scss`:
- Around line 1-11: This SCSS file is missing the compat mixins import and
top-level override mixin; add the import statement for the compat mixins module
using the namespace compatMixins (i.e. `@use`
'libs/components/theme/src/lib/styles/compat-tokens-mixins' as compatMixins) at
the top of modal-banner-content.component.scss and invoke the top-level mixin
compatMixins.sky-default-overrides outside any selector blocks (before
.banner-content) so the overrides apply globally to this component.
In `@apps/playground/src/app/components/modal/modal-banner-content.component.ts`:
- Around line 4-8: The component decorator for ModalBannerContentComponent lacks
the OnPush change-detection setting; update the `@Component` on
ModalBannerContentComponent to include changeDetection:
ChangeDetectionStrategy.OnPush and add the corresponding import for
ChangeDetectionStrategy from '@angular/core' so the component uses OnPush
semantics. Ensure you update the imports section to include
ChangeDetectionStrategy and leave the rest of the decorator intact.
In `@apps/playground/src/app/components/modal/modal-banner-image.component.ts`:
- Around line 4-7: The component decorator for ModalBannerImageComponent should
opt into OnPush change detection: import ChangeDetectionStrategy from
'@angular/core' and add changeDetection: ChangeDetectionStrategy.OnPush to the
`@Component` metadata (the decorator that currently contains imports and
templateUrl) so the stateless component uses the repo default strategy.
In `@apps/playground/src/app/components/modal/modal-visual.component.html`:
- Around line 145-158: The two buttons that open banner modals reuse the generic
test hook class and should get unique hooks: update the button element for
openBannerImageModal() to use a distinct test hook (e.g., replace or add a class
like "sky-test-modal-banner-image") and update the button for
openBannerContentModal() to use a different distinct test hook (e.g.,
"sky-test-modal-banner-content"); ensure you change the class attribute on the
button elements that call openBannerImageModal() and openBannerContentModal() so
automation can target each launcher deterministically.
In
`@libs/components/code-examples/src/lib/modules/modals/modal/with-banner/example.component.ts`:
- Around line 1-18: Update the `@Component` metadata to use the "with-banner"
variant selector and enable OnPush change detection: change the selector string
from 'app-modals-modal-basic-with-controller-example' to
'app-modals-modal-basic-with-banner-example', import ChangeDetectionStrategy
from '@angular/core', and add changeDetection: ChangeDetectionStrategy.OnPush to
the `@Component` decorator; ensure the component class that currently implements
OnDestroy remains unchanged (no runtime behavior changes required).
In `@libs/components/modals/src/lib/modules/modal/modal-banner.component.scss`:
- Around line 1-9: Add the required compat mixins scaffold at the top of
modal-banner.component.scss by adding the import `@use`
'libs/components/theme/src/lib/styles/compat-tokens-mixins' as compatMixins; and
then invoking/declaring the empty compat mixin hooks (e.g.,
compatMixins.override and compatMixins.override-top or the repo-standard mixin
names) immediately below the import and before the existing :host selector so
the component participates in the standard override path.
In `@libs/components/modals/src/lib/modules/modal/modal-content.component.scss`:
- Around line 21-25: Add an override CSS variable for the banner/content spacing
and use it as the padding-top fallback: inside the existing mixin that defines
component tokens, create a new variable (e.g.
--sky-comp-modal-banner-content-space-inset) and initialize it to the current
calc(...) expression using the existing tokens
var(--sky-comp-modal-header-space-inset-bottom) and
var(--sky-comp-modal-content-space-inset-top); then replace the hard-coded
padding-top in the sky-modal-banner + sky-modal-content rule with padding-top:
var(--sky-comp-modal-banner-content-space-inset, calc(...)). Ensure the new
variable is exposed so consumers can override via sky-default-overrides and
reference the same tokens used originally.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e2889815-f020-4e4d-ba39-67c338935b3c
📒 Files selected for processing (36)
apps/e2e/modals-storybook-e2e/src/e2e/modal.component.cy.tsapps/e2e/modals-storybook/src/app/modal/modal.component.htmlapps/e2e/modals-storybook/src/app/modal/modal.component.tsapps/e2e/modals-storybook/src/app/modal/modals/modal-basic.component.htmlapps/e2e/modals-storybook/src/app/modal/modals/modal-context.tsapps/playground/src/app/components/modal/modal-banner-content.component.htmlapps/playground/src/app/components/modal/modal-banner-content.component.scssapps/playground/src/app/components/modal/modal-banner-content.component.tsapps/playground/src/app/components/modal/modal-banner-image.component.htmlapps/playground/src/app/components/modal/modal-banner-image.component.tsapps/playground/src/app/components/modal/modal-visual.component.htmlapps/playground/src/app/components/modal/modal-visual.component.tslibs/components/code-examples/routes/src/index.tslibs/components/code-examples/src/index.tslibs/components/code-examples/src/lib/modules/modals/modal/with-banner/example.component.tslibs/components/code-examples/src/lib/modules/modals/modal/with-banner/modal.component.tslibs/components/modals/documentation.jsonlibs/components/modals/src/index.tslibs/components/modals/src/lib/modules/modal/fixtures/modal-banner.component.fixture.htmllibs/components/modals/src/lib/modules/modal/fixtures/modal-banner.component.fixture.tslibs/components/modals/src/lib/modules/modal/fixtures/modal-context.tslibs/components/modals/src/lib/modules/modal/fixtures/modal-fixtures.module.tslibs/components/modals/src/lib/modules/modal/fixtures/modal.component.fixture.htmllibs/components/modals/src/lib/modules/modal/modal-banner.component.htmllibs/components/modals/src/lib/modules/modal/modal-banner.component.scsslibs/components/modals/src/lib/modules/modal/modal-banner.component.spec.tslibs/components/modals/src/lib/modules/modal/modal-banner.component.tslibs/components/modals/src/lib/modules/modal/modal-content.component.scsslibs/components/modals/src/lib/modules/modal/modal.component.htmllibs/components/modals/src/lib/modules/modal/modal.component.spec.tslibs/components/modals/src/lib/modules/modal/modal.component.tslibs/components/modals/src/lib/modules/modal/modal.module.tslibs/components/modals/testing/src/modules/modal/modal-banner-harness.spec.tslibs/components/modals/testing/src/modules/modal/modal-banner-harness.tslibs/components/modals/testing/src/modules/modal/modal-harness.tslibs/components/modals/testing/src/public-api.ts
libs/components/code-examples/src/lib/modules/modals/modal/with-banner/modal.component.ts
Show resolved
Hide resolved
libs/components/modals/testing/src/modules/modal/modal-banner-harness.spec.ts
Show resolved
Hide resolved
libs/components/modals/testing/src/modules/modal/modal-banner-harness.spec.ts
Show resolved
Hide resolved
libs/components/modals/testing/src/modules/modal/modal-banner-harness.ts
Show resolved
Hide resolved
Collaborator
Author
…f620b54d_1773169838236
Blackbaud-PaulCrowder
temporarily deployed
to
e2e-team-members
GitHub Actions
Inactive
johnhwhite
approved these changes
Mar 12, 2026
Blackbaud-PaulCrowder
deleted the
cherry-pick_c6cf820017fcb46232234cba0d06011af620b54d_1773169838236
branch
johnhwhite
pushed a commit
that referenced
this pull request
Mar 17, 2026
## [14.0.0-alpha.9](14.0.0-alpha.8...14.0.0-alpha.9) (2026-03-17) ### ⚠ BREAKING CHANGES * **components/indicators:** replace `@angular/animations` in tokens component with CSS transitions (#4303) * **components/tabs:** replace `@angular/animations` with CSS transitions (#4288) * **components/flyout:** replace `@angular/animations` with CSS transitions (#4285) ### Features * **components/lists:** deprecate filter summary ([#4307](#4307)) ([b5567d9](b5567d9)), closes [AB#3611503](https://dev.azure.com/blackbaud/Products/_workitems/edit/3611503) * **components/modals:** add modal banner ([#4275](#4275)) ([#4280](#4280)) ([b186fc2](b186fc2)) ### Bug Fixes * **components/core:** transition handler emits via microtask when animations disabled ([#4304](#4304)) ([3b9cd2d](3b9cd2d)) * **components/data-manager:** address data mutation during view updates ([#4309](#4309)) ([#4311](#4311)) ([6cadb59](6cadb59)), closes [AB#3705639](https://dev.azure.com/blackbaud/Products/_workitems/edit/3705639) * **components/flyout:** replace `@angular/animations` with CSS transitions ([#4285](#4285)) ([96aa341](96aa341)) * **components/indicators:** replace `@angular/animations` in tokens component with CSS transitions ([#4303](#4303)) ([bcb9d88](bcb9d88)) * **components/tabs:** replace `@angular/animations` with CSS transitions ([#4288](#4288)) ([90b183b](90b183b)) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Breaking Changes** * Indicators, tabs, and flyout components now use CSS transitions instead of animations. * **New Features** * Added modal banner component. * **Deprecations** * Filter summary in list components deprecated. * **Bug Fixes** * Animation behavior when disabled, data handling during updates, and transition styling improvements. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
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
3 participants
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.
🍒 Cherry picked from #4275 feat(components/modals): add modal banner
Summary by CodeRabbit