Skip to content

fix(components/lookup)!: replace @angular/animations with CSS transitions for search component#4319

Merged
Blackbaud-SteveBrush merged 18 commits intomainfrom
search-animation
Mar 19, 2026
Merged

fix(components/lookup)!: replace @angular/animations with CSS transitions for search component#4319
Blackbaud-SteveBrush merged 18 commits intomainfrom
search-animation

Conversation

@Blackbaud-SteveBrush
Copy link
Copy Markdown
Member

@Blackbaud-SteveBrush Blackbaud-SteveBrush commented Mar 18, 2026

BREAKING CHANGE

SkySearchComponent has replaced @angular/animations with CSS transitions. Tests that interact with search may need to add provideNoopSkyAnimations() from @skyux/core to their TestBed providers to disable SKY UX CSS transitions during tests.

import { provideNoopSkyAnimations } from '@skyux/core';

TestBed.configureTestingModule({
  providers: [provideNoopSkyAnimations()],
});

AB#3913631

Summary by CodeRabbit

Release Notes

  • Refactor

    • Search input expand/collapse behavior now uses CSS transitions for improved performance and consistency.
  • Tests

    • Updated test configurations to align with new transition-based animation approach across lookup and search components.
  • Chores

    • Removed Angular animations dependency from the lookup component package.

@Blackbaud-SteveBrush Blackbaud-SteveBrush added the risk level (author): 2 This change has a slight chance of introducing a bug label Mar 18, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 18, 2026

📝 Walkthrough

Walkthrough

This PR migrates the search component from Angular's animation system to CSS-based transitions with SkyUX's transition-end handler directive. It removes animation trigger logic, simplifies state management with a boolean flag, updates tests to use provideNoopSkyAnimations(), and removes @angular/animations from peer dependencies. A minor refactor in the repeater component also switches from setTimeout to RxJS delay().

Changes

Cohort / File(s) Summary
Search Component Animation Migration
search.component.ts, search.component.html, search.component.scss
Removed Angular animation trigger (@inputState), replaced with CSS transitions and inputShown boolean state. Added onInputTransitionEnd() handler, replaced animation-based focus timing with afterNextRender(), and added .sky-search-input-hidden CSS class for opacity/width collapse.
Search Service Cleanup
search-adapter.service.ts, search.module.ts
Removed Renderer2 dependency and animation methods (startInputAnimation, endInputAnimation) from adapter service. Added _SkyTransitionEndHandlerDirective import to module for transition-end handling support.
Search Component Testing
search.component.spec.ts, search-fixture.spec.ts, search-testing.module.ts, search-harness.spec.ts
Replaced NoopAnimationsModule with provideNoopSkyAnimations() provider in test setup across all search testing files, aligning with noop animation handling strategy.
E2E and Example Tests
example.component.spec.ts, search.cy.ts, page-data-manager-split-view.cy.ts
Updated test setup to use provideNoopSkyAnimations() in example spec. Modified mobile E2E assertions to check element visibility before screenshots and added conditional mobile viewport checks in split-view test.
Library Configuration
package.json (lookup)
Removed @angular/animations from peerDependencies, eliminating the requirement for consumers to provide Angular animations package.
Repeater Component Refactor
repeater.component.ts
Moved subscription deferral mechanism from explicit setTimeout to RxJS delay(0) in the items-changes subscription pipeline.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • johnhwhite
  • Blackbaud-TrevorBurch

Poem

🐰 From angular twirls to CSS flows,
The search input hides and grows,
Transition-end directives now guide the way,
No more animations to delay,
Simpler, faster, hip-hop-hooray! 🎉

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: replacing Angular animations with CSS transitions in the search component, and includes the breaking change indicator (!).

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch search-animation
📝 Coding Plan
  • Generate coding plan for human review comments

Comment @coderabbitai help to get the list of available commands and usage tips.

@nx-cloud
Copy link
Copy Markdown

nx-cloud bot commented Mar 18, 2026

View your CI Pipeline Execution ↗ for commit 111ee65

Command Status Duration Result
nx build code-examples-playground --baseHref=ht... ✅ Succeeded 4m 30s View ↗
nx build playground --baseHref=https://blackbau... ✅ Succeeded 2m 22s View ↗
nx build integration --baseHref=https://blackba... ✅ Succeeded 53s View ↗

☁️ Nx Cloud last updated this comment at 2026-03-19 18:06:40 UTC

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

This PR migrates the lookup SkySearchComponent away from @angular/animations and onto CSS transitions, using SKY UX’s transition-end handling utilities to keep behavior testable when CSS transitions are disabled.

Changes:

  • Replaces Angular animation trigger usage with CSS transition styles and _SkyTransitionEndHandlerDirective-driven transitionEnd handling.
  • Refactors component state to signals/computed values for collapse/full-width logic and mobile open/close behavior.
  • Updates unit tests and code-example tests to use provideNoopSkyAnimations() instead of NoopAnimationsModule.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
libs/components/lookup/src/lib/modules/search/search.module.ts Imports _SkyTransitionEndHandlerDirective so the template can listen for CSS transition completion.
libs/components/lookup/src/lib/modules/search/search.component.ts Removes Angular animations and reworks open/close logic around signals, CSS transitions, and afterNextRender focusing.
libs/components/lookup/src/lib/modules/search/search.component.html Replaces [@inputState] bindings with skyTransitionEndHandler + class bindings.
libs/components/lookup/src/lib/modules/search/search.component.scss Adds opacity/width transition rules and hidden-state styling for the input container.
libs/components/lookup/src/lib/modules/search/search.component.spec.ts Switches test setup to provideNoopSkyAnimations() to stabilize CSS-transition-driven behavior.
libs/components/lookup/src/lib/modules/search/search-adapter.service.ts Removes animation-related DOM styling logic that was tied to Angular animations.
libs/components/code-examples/src/lib/modules/lookup/search/basic/example.component.spec.ts Updates example test to disable SKY UX CSS transitions via provideNoopSkyAnimations().

@blackbaud-sky-build-user
Copy link
Copy Markdown
Collaborator

blackbaud-sky-build-user commented Mar 18, 2026

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (1)
libs/components/lookup/src/lib/modules/search/search.component.html (1)

17-19: Use a direct class binding for this boolean toggle.

[class.sky-search-dismiss-absolute] is the repo-preferred pattern here and fits the new signal-based bindings cleanly.

♻️ Suggested template change
-    [ngClass]="{
-      'sky-search-dismiss-absolute': mobileSearchShown() || isFullWidth()
-    }"
+    [class.sky-search-dismiss-absolute]="mobileSearchShown() || isFullWidth()"

As per coding guidelines, "Do NOT use ngClass; use class bindings instead in Angular templates."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@libs/components/lookup/src/lib/modules/search/search.component.html` around
lines 17 - 19, Replace the ngClass boolean object binding with a direct class
binding: remove the [ngClass] usage in the search.component.html and add a
boolean class binding using the same condition (mobileSearchShown() ||
isFullWidth()) so the element toggles the CSS class
"sky-search-dismiss-absolute" directly; locate the template expression around
the search component where [ngClass] is used and update it to use
[class.sky-search-dismiss-absolute] with the existing mobileSearchShown() and
isFullWidth() signals.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@libs/components/code-examples/src/lib/modules/lookup/search/basic/example.component.spec.ts`:
- Line 3: The test is missing Angular's NoopAnimationsModule which can cause
animation timing issues; import NoopAnimationsModule from
'@angular/platform-browser/animations' and add it to the
TestBed.configureTestingModule imports in example.component.spec.ts (alongside
any existing provideNoopSkyAnimations() usage) so the test uses Angular's noop
animation engine (look for the TestBed.configureTestingModule call in this spec
and add NoopAnimationsModule to its imports array).

In `@libs/components/lookup/src/lib/modules/search/search.component.ts`:
- Around line 141-151: The breakpoint-driven UI signals (inputShown,
mobileSearchShown, searchButtonShown) can become stale when expandMode changes
at runtime; update the component so these three signals are derived reactively
from both the current breakpoint and expandMode instead of only from the
one-time subscription in ngOnInit: either implement an effect that watches
expandMode() and the breakpoint value and sets
inputShown/mobileSearchShown/searchButtonShown accordingly, or always subscribe
to breakpoints in ngOnInit and keep the existing guard logic inside
mediaQueryCallback() so changes to expandMode (used by isCollapsible and
isFullWidth) immediately recompute the mobile state; reference the computed
getters isCollapsible/isFullWidth, the signals
inputShown/mobileSearchShown/searchButtonShown, the expandMode input, ngOnInit,
and mediaQueryCallback when making the change.

---

Nitpick comments:
In `@libs/components/lookup/src/lib/modules/search/search.component.html`:
- Around line 17-19: Replace the ngClass boolean object binding with a direct
class binding: remove the [ngClass] usage in the search.component.html and add a
boolean class binding using the same condition (mobileSearchShown() ||
isFullWidth()) so the element toggles the CSS class
"sky-search-dismiss-absolute" directly; locate the template expression around
the search component where [ngClass] is used and update it to use
[class.sky-search-dismiss-absolute] with the existing mobileSearchShown() and
isFullWidth() signals.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 7a703cea-85ac-4a4e-91b5-dc96765a9503

📥 Commits

Reviewing files that changed from the base of the PR and between bb4a3c7 and 1c0d173.

📒 Files selected for processing (7)
  • libs/components/code-examples/src/lib/modules/lookup/search/basic/example.component.spec.ts
  • libs/components/lookup/src/lib/modules/search/search-adapter.service.ts
  • libs/components/lookup/src/lib/modules/search/search.component.html
  • libs/components/lookup/src/lib/modules/search/search.component.scss
  • libs/components/lookup/src/lib/modules/search/search.component.spec.ts
  • libs/components/lookup/src/lib/modules/search/search.component.ts
  • libs/components/lookup/src/lib/modules/search/search.module.ts

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
apps/e2e/lookup-storybook-e2e/src/e2e/search.component.cy.ts (1)

37-51: ⚠️ Potential issue | 🟡 Minor

Duplicate test description detected.

Both tests at lines 23 and 37 share the identical description "should render the component collapsed on mobile", but they test different states—the first tests the collapsed state, while this one tests the expanded ("open") state. This could cause confusion in test reports and some test runners may exhibit unexpected behavior with duplicate names.

📝 Suggested fix
-      it('should render the component collapsed on mobile', () => {
+      it('should render the component expanded on mobile', () => {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/e2e/lookup-storybook-e2e/src/e2e/search.component.cy.ts` around lines 37
- 51, The test description string for the second "it" block currently reads
"should render the component collapsed on mobile" but the test actually opens
the search (clicks .sky-search-btn-open) and captures the expanded state; update
the description in that it(...) to something like "should render the component
expanded (open) on mobile" to avoid duplicate test names and reflect the
behavior being tested (locate the second it(...) in search.component.cy.ts that
contains the clicks on '#filled-search .sky-search-btn-open' and '#empty-search
.sky-search-btn-open' and change the first argument accordingly).
🧹 Nitpick comments (1)
libs/components/lookup/src/lib/modules/search/search.component.scss (1)

35-37: Consider using a design system variable for the timing function.

Line 36 uses var(--sky-transition-time-short) for duration, but line 37 hardcodes ease for the timing function. For consistency with the design system patterns, consider using var(--sky-ease) if available.

♻️ Suggested change
     transition-property: opacity, width;
     transition-duration: var(--sky-transition-time-short);
-    transition-timing-function: ease;
+    transition-timing-function: var(--sky-ease, ease);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@libs/components/lookup/src/lib/modules/search/search.component.scss` around
lines 35 - 37, The transition-timing-function is hardcoded to "ease" in
search.component.scss; replace that literal with the design-system variable
(e.g., var(--sky-ease)) so timing uses the shared token; update the rule
containing transition-property/transition-duration/transition-timing-function
(the transition-timing-function declaration in search.component.scss) to use
var(--sky-ease) and optionally provide a safe fallback like var(--sky-ease,
ease) if you want to preserve behavior when the variable is missing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@apps/e2e/lookup-storybook-e2e/src/e2e/search.component.cy.ts`:
- Around line 37-51: The test description string for the second "it" block
currently reads "should render the component collapsed on mobile" but the test
actually opens the search (clicks .sky-search-btn-open) and captures the
expanded state; update the description in that it(...) to something like "should
render the component expanded (open) on mobile" to avoid duplicate test names
and reflect the behavior being tested (locate the second it(...) in
search.component.cy.ts that contains the clicks on '#filled-search
.sky-search-btn-open' and '#empty-search .sky-search-btn-open' and change the
first argument accordingly).

---

Nitpick comments:
In `@libs/components/lookup/src/lib/modules/search/search.component.scss`:
- Around line 35-37: The transition-timing-function is hardcoded to "ease" in
search.component.scss; replace that literal with the design-system variable
(e.g., var(--sky-ease)) so timing uses the shared token; update the rule
containing transition-property/transition-duration/transition-timing-function
(the transition-timing-function declaration in search.component.scss) to use
var(--sky-ease) and optionally provide a safe fallback like var(--sky-ease,
ease) if you want to preserve behavior when the variable is missing.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 44bc9bc8-04a0-48bb-97c1-3713daaf9cd3

📥 Commits

Reviewing files that changed from the base of the PR and between 1c0d173 and c65904d.

📒 Files selected for processing (7)
  • apps/e2e/lookup-storybook-e2e/src/e2e/search.component.cy.ts
  • libs/components/lookup/package.json
  • libs/components/lookup/src/lib/modules/search/search.component.html
  • libs/components/lookup/src/lib/modules/search/search.component.scss
  • libs/components/lookup/src/lib/modules/search/search.component.ts
  • libs/components/lookup/testing/src/legacy/search/search-testing.module.ts
  • libs/components/lookup/testing/src/modules/search/search-harness.spec.ts
💤 Files with no reviewable changes (1)
  • libs/components/lookup/package.json
🚧 Files skipped from review as they are similar to previous changes (1)
  • libs/components/lookup/src/lib/modules/search/search.component.html

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

This PR migrates SkySearchComponent in @skyux/lookup from @angular/animations to CSS transitions, aligning the component with SKY UX’s CSS-motion infrastructure and updating related tests to disable CSS transitions via provideNoopSkyAnimations().

Changes:

  • Replaced Angular animation triggers with CSS transitions and _SkyTransitionEndHandlerDirective to drive post-transition behavior.
  • Simplified the search adapter by removing animation-specific DOM style manipulation.
  • Updated unit/harness/code-example tests (and one e2e spec) to use provideNoopSkyAnimations() instead of NoopAnimationsModule.

Reviewed changes

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

Show a summary per file
File Description
libs/components/lookup/src/lib/modules/search/search.component.ts Removes Angular animations and implements transition-driven state updates + focus timing via afterNextRender.
libs/components/lookup/src/lib/modules/search/search.component.html Wires skyTransitionEndHandler into the template to react to CSS transition completion.
libs/components/lookup/src/lib/modules/search/search.component.scss Adds opacity/width CSS transitions and a hidden state class for the input container.
libs/components/lookup/src/lib/modules/search/search.module.ts Imports _SkyTransitionEndHandlerDirective so the template directive is available in the module.
libs/components/lookup/src/lib/modules/search/search-adapter.service.ts Removes renderer-based animation helpers; retains input focus/select helpers.
libs/components/lookup/src/lib/modules/search/search.component.spec.ts Switches from NoopAnimationsModule to provideNoopSkyAnimations() for CSS-transition stability in tests.
libs/components/lookup/testing/src/modules/search/search-harness.spec.ts Adds provideNoopSkyAnimations() to stabilize harness tests against CSS transitions.
libs/components/lookup/testing/src/legacy/search/search-testing.module.ts Replaces NoopAnimationsModule export with provideNoopSkyAnimations() provider for the legacy testing module.
libs/components/lookup/package.json Removes @angular/animations from peer dependencies.
libs/components/code-examples/src/lib/modules/lookup/search/basic/example.component.spec.ts Updates code example test to use provideNoopSkyAnimations().
apps/e2e/lookup-storybook-e2e/src/e2e/search.component.cy.ts Adjusts readiness/screenshot flow for the search story under CSS transitions.
Comments suppressed due to low confidence (1)

apps/e2e/lookup-storybook-e2e/src/e2e/search.component.cy.ts:44

  • After clicking the mobile open buttons, the test takes a screenshot immediately. With the new CSS transition + transitionEnd handler behavior, the DOM state may still be mid-transition/microtask update when the screenshot is captured. Add a post-click assertion that the open state has settled (e.g., dismiss UI visible / open button hidden) before the screenshot/percy snapshot to reduce flakiness.
      it('should render the component collapsed on mobile', () => {
        cy.skyReady('app-search');
        cy.viewport(E2eVariations.MOBILE_WIDTHS[0], 800);
        cy.get('#filled-search .sky-search-btn-open').click();
        cy.get('#empty-search .sky-search-btn-open').click();
        cy.get('app-search').screenshot(
          `searchcomponent-search--search-${theme}-mobile-open`,
        );

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

This PR migrates SkySearchComponent (lookup search) from @angular/animations to CSS transitions, updates tests to use provideNoopSkyAnimations() for deterministic behavior, and removes the @angular/animations peer dependency from the lookup package.

Changes:

  • Replace Angular animation trigger/state logic with CSS transition + skyTransitionEndHandler to coordinate UI state updates.
  • Update unit tests, harness tests, and code example tests to disable SKY UX CSS transitions via provideNoopSkyAnimations().
  • Remove @angular/animations peer dependency from @skyux/lookup and adjust a few e2e assertions around the collapsed mobile UI.

Reviewed changes

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

Show a summary per file
File Description
libs/components/lookup/src/lib/modules/search/search.component.ts Replaces Angular animation-driven state with inputShown boolean + transition-end handling.
libs/components/lookup/src/lib/modules/search/search.component.html Swaps [@inputState] for skyTransitionEndHandler bindings and hidden-class toggling.
libs/components/lookup/src/lib/modules/search/search.component.scss Adds CSS transitions and hidden-state styles for the input container.
libs/components/lookup/src/lib/modules/search/search.module.ts Imports transition-end handler directive needed by the template.
libs/components/lookup/src/lib/modules/search/search-adapter.service.ts Removes animation-specific DOM measurements/styles and Renderer2 dependency.
libs/components/lookup/src/lib/modules/search/search.component.spec.ts Updates tests to use provideNoopSkyAnimations() instead of NoopAnimationsModule.
libs/components/lookup/testing/src/modules/search/search-harness.spec.ts Disables CSS transitions for harness stability.
libs/components/lookup/testing/src/legacy/search/search-testing.module.ts Replaces NoopAnimationsModule with provideNoopSkyAnimations() for legacy testing module.
libs/components/code-examples/src/lib/modules/lookup/search/basic/example.component.spec.ts Updates example test to disable CSS transitions via provideNoopSkyAnimations().
libs/components/lookup/package.json Removes @angular/animations from peer dependencies.
apps/e2e/lookup-storybook-e2e/src/e2e/search.component.cy.ts Adds explicit readiness + mobile collapsed-state assertions before screenshots/interactions.
apps/integration-e2e/src/e2e/page-data-manager-split-view.cy.ts Adds a mobile-only assertion for the collapsed search open button.
libs/components/lists/src/lib/modules/repeater/repeater.component.ts Replaces an inner setTimeout with delay(0) while keeping the “timeout hack” behavior.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

♻️ Duplicate comments (1)
libs/components/lookup/src/lib/modules/search/search.component.ts (1)

189-196: ⚠️ Potential issue | 🟠 Major

expandMode can still leave the mobile state stale after init.

The breakpoint listener is still only wired when the component starts in a collapsible mode, and this branch only updates isCollapsible/isFullWidth. If expandMode flips between 'responsive' and 'none'/'fit' on an xs viewport, inputShown, mobileSearchShown, and searchButtonShown can stay in the old state and future breakpoint changes may never be observed.

Also applies to: 201-217

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@libs/components/lookup/src/lib/modules/search/search.component.ts` around
lines 189 - 196, ngOnInit currently only wires the
`#mediaQuerySvc.breakpointChange` when `#searchShouldCollapse`() is true which
leaves inputShown, mobileSearchShown and searchButtonShown stale if expandMode
later flips to/from 'responsive'; always subscribe to
`#mediaQuerySvc.breakpointChange` in ngOnInit (regardless of
`#searchShouldCollapse`()) and call `#mediaQueryCallback` on events so breakpoint
changes are always observed, and ensure any expandMode change handler (or code
paths that flip expandMode) refreshes computed state (isCollapsible,
isFullWidth, inputShown, mobileSearchShown, searchButtonShown) or invokes the
same update routine used by `#mediaQueryCallback` so view flags are recalculated
whenever expandMode toggles between 'responsive' and 'none'|'fit'.
🤖 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/lookup-storybook-e2e/src/e2e/search.component.cy.ts`:
- Around line 39-45: The test clicks both search open buttons but only waits for
the filled search to stabilize, so ensure the empty search is also fully
expanded before taking the screenshot by waiting for its input container to
finish its CSS transition; after clicking '#empty-search .sky-search-btn-open'
add an assertion that the corresponding '.sky-search-input-container' is visible
(or has the expected width/opacity) inside the '#empty-search' scope (use
selectors like '#empty-search .sky-search-input-container') so that the later
cy.get('app-search').screenshot(...) captures both searches in their final
expanded state.

In `@libs/components/lookup/src/lib/modules/search/search.component.ts`:
- Around line 294-300: The onInputTransitionEnd handler hides the mobile search
UI but doesn't restore keyboard focus to the opener, so update
onInputTransitionEnd to set focus back to the opener when you show the open
button: after you set searchButtonShown = true and mobileSearchShown = false,
call focus() on the opener element (the element that toggles the search/open
button) — locate the opener element reference used in this component (e.g., a
ViewChild or element ref associated with the search button, such as
searchButton, searchToggleButton, or similar) and call its focus method
(guarding with a null check) so keyboard users are returned to the control; keep
this logic inside onInputTransitionEnd and only run when you have just collapsed
the mobile search (i.e., when `#breakpoint`() === 'xs' &&
`#searchShouldCollapse`()).

In `@libs/components/lookup/testing/src/legacy/search/search-testing.module.ts`:
- Around line 2-10: The testing module currently only provides
provideNoopSkyAnimations() but no longer exports NoopAnimationsModule, so
restore suppression of Angular animations by importing NoopAnimationsModule from
'@angular/platform-browser/animations' and adding NoopAnimationsModule to the
`@NgModule` exports array (alongside SkySearchModule) in the SkySearch testing
module; leave the existing providers (provideNoopSkyAnimations()) unchanged.

---

Duplicate comments:
In `@libs/components/lookup/src/lib/modules/search/search.component.ts`:
- Around line 189-196: ngOnInit currently only wires the
`#mediaQuerySvc.breakpointChange` when `#searchShouldCollapse`() is true which
leaves inputShown, mobileSearchShown and searchButtonShown stale if expandMode
later flips to/from 'responsive'; always subscribe to
`#mediaQuerySvc.breakpointChange` in ngOnInit (regardless of
`#searchShouldCollapse`()) and call `#mediaQueryCallback` on events so breakpoint
changes are always observed, and ensure any expandMode change handler (or code
paths that flip expandMode) refreshes computed state (isCollapsible,
isFullWidth, inputShown, mobileSearchShown, searchButtonShown) or invokes the
same update routine used by `#mediaQueryCallback` so view flags are recalculated
whenever expandMode toggles between 'responsive' and 'none'|'fit'.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 24d32bef-f6cb-4c8e-9705-bfe24148b4c5

📥 Commits

Reviewing files that changed from the base of the PR and between 1c0d173 and 72ce6e2.

📒 Files selected for processing (9)
  • apps/e2e/lookup-storybook-e2e/src/e2e/search.component.cy.ts
  • apps/integration-e2e/src/e2e/page-data-manager-split-view.cy.ts
  • libs/components/lists/src/lib/modules/repeater/repeater.component.ts
  • libs/components/lookup/package.json
  • libs/components/lookup/src/lib/modules/search/search.component.html
  • libs/components/lookup/src/lib/modules/search/search.component.scss
  • libs/components/lookup/src/lib/modules/search/search.component.ts
  • libs/components/lookup/testing/src/legacy/search/search-testing.module.ts
  • libs/components/lookup/testing/src/modules/search/search-harness.spec.ts
💤 Files with no reviewable changes (1)
  • libs/components/lookup/package.json
✅ Files skipped from review due to trivial changes (2)
  • libs/components/lookup/testing/src/modules/search/search-harness.spec.ts
  • libs/components/lists/src/lib/modules/repeater/repeater.component.ts

@Blackbaud-SteveBrush Blackbaud-SteveBrush marked this pull request as ready for review March 19, 2026 17:15
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 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/lookup-storybook-e2e/src/e2e/search.component.cy.ts`:
- Line 38: The test description "should render the component collapsed on
mobile" is incorrect because the test actually clicks the open buttons and
verifies the expanded state; update the test to either (A) change the it(...)
description to "should render the component expanded/open on mobile" to match
the actions, or (B) if the intent was to test the collapsed state, remove the
click interactions and assert the collapsed UI instead—locate the offending it
block in search.component.cy.ts and adjust its description or its
interactions/assertions accordingly.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 086ae323-824a-46d6-a43c-0360ffe87385

📥 Commits

Reviewing files that changed from the base of the PR and between 1c0d173 and 397bb45.

📒 Files selected for processing (10)
  • apps/e2e/lookup-storybook-e2e/src/e2e/search.component.cy.ts
  • apps/integration-e2e/src/e2e/page-data-manager-split-view.cy.ts
  • libs/components/lists/src/lib/modules/repeater/repeater.component.ts
  • libs/components/lookup/package.json
  • libs/components/lookup/src/lib/modules/search/search.component.html
  • libs/components/lookup/src/lib/modules/search/search.component.scss
  • libs/components/lookup/src/lib/modules/search/search.component.ts
  • libs/components/lookup/testing/src/legacy/search/search-fixture.spec.ts
  • libs/components/lookup/testing/src/legacy/search/search-testing.module.ts
  • libs/components/lookup/testing/src/modules/search/search-harness.spec.ts
💤 Files with no reviewable changes (1)
  • libs/components/lookup/package.json
✅ Files skipped from review due to trivial changes (1)
  • libs/components/lookup/testing/src/legacy/search/search-fixture.spec.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • libs/components/lookup/src/lib/modules/search/search.component.html

@johnhwhite johnhwhite self-assigned this Mar 19, 2026
@Blackbaud-SteveBrush Blackbaud-SteveBrush enabled auto-merge (squash) March 19, 2026 17:56
@Blackbaud-SteveBrush Blackbaud-SteveBrush merged commit 2dec706 into main Mar 19, 2026
49 checks passed
@Blackbaud-SteveBrush Blackbaud-SteveBrush deleted the search-animation branch March 19, 2026 18:10
johnhwhite pushed a commit that referenced this pull request Mar 19, 2026
##
[14.0.0-alpha.11](14.0.0-alpha.10...14.0.0-alpha.11)
(2026-03-19)


### ⚠ BREAKING CHANGES

* **components/lookup:** replace `@angular/animations` with CSS
transitions for search component (#4319)

### Bug Fixes

* **components/lookup:** replace `@angular/animations` with CSS
transitions for search component
([#4319](#4319))
([2dec706](2dec706))
* **components/phone-field:** remove `@angular/animations` from the
phone field component
([#4321](#4321))
([00051ae](00051ae))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk level (author): 2 This change has a slight chance of introducing a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants