Skip to content

refactor(live-preview): migrate live-preview to signals#2278

Open
spliffone wants to merge 1 commit into
mainfrom
refactor/live-previewer-to-onpush
Open

refactor(live-preview): migrate live-preview to signals#2278
spliffone wants to merge 1 commit into
mainfrom
refactor/live-previewer-to-onpush

Conversation

@spliffone

@spliffone spliffone commented Jul 3, 2026

Copy link
Copy Markdown
Member

@spliffone spliffone requested review from a team as code owners July 3, 2026 12:28
@spliffone spliffone marked this pull request as draft July 3, 2026 12:28

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the SiLivePreviewComponent to adopt Angular's signal-based inputs (input and model) instead of the traditional @input decorators, updating both the component class and its HTML template. The review feedback recommends migrating the remaining ticketBaseUrl input to the signal-based input() function to fully comply with the repository style guide. Additionally, the reviewer suggests caching signal reads (such as this.example() and this.baseUrl()) in local variables across multiple methods to avoid redundant evaluations and improve code readability.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread projects/live-preview/components/si-live-preview/si-live-preview.component.ts Outdated
Comment thread projects/live-preview/components/si-live-preview/si-live-preview.component.ts Outdated
@dr-itz

dr-itz commented Jul 3, 2026

Copy link
Copy Markdown
Member

PR title is confusing. This is a signal migration (which is fine), not making in OnPush. We cannot mark live-preview as OnPush as it will break any examples that rely on Default. Everything is marked as Default and it should stay that way, at least for a few releases.

Having signals is the right thing to do as stuff will work in either case.

@spliffone spliffone force-pushed the refactor/live-previewer-to-onpush branch 5 times, most recently from e9fb7b7 to 1fe43e7 Compare July 5, 2026 19:00
@spliffone spliffone changed the title refactor(live-preview): migrate live-preview to OnPush refactor(live-preview): migrate live-preview to signals Jul 5, 2026
@spliffone spliffone marked this pull request as ready for review July 5, 2026 19:12
BREAKING CHANGE: The live-preview components were migrated to signal inputs.
@spliffone spliffone force-pushed the refactor/live-previewer-to-onpush branch from 1fe43e7 to 0b709ac Compare July 5, 2026 19:17
@spliffone spliffone added the breaking-changes Marks issues and PRs that are breaking the API label Jul 5, 2026
@spliffone spliffone added this to the 51.0.0 milestone Jul 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking-changes Marks issues and PRs that are breaking the API

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants