Skip to content

Comments

fix(landing-page): pass loginAlert action and translationParams#1556

Open
ljanner wants to merge 1 commit intomainfrom
fix/landing-page-login-alert
Open

fix(landing-page): pass loginAlert action and translationParams#1556
ljanner wants to merge 1 commit intomainfrom
fix/landing-page-login-alert

Conversation

@ljanner
Copy link
Member

@ljanner ljanner commented Feb 19, 2026

Resolves #1555


@ljanner ljanner added the bug Something isn't working label Feb 19, 2026
@ljanner ljanner added this to the 49.x milestone Feb 19, 2026
@ljanner ljanner requested a review from dauriamarco February 19, 2026 16:28
@ljanner ljanner marked this pull request as ready for review February 19, 2026 16:28
@ljanner ljanner requested review from a team as code owners February 19, 2026 16:28
Copy link
Contributor

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

Choose a reason for hiding this comment

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

Code Review

The pull request correctly implements the passing of action and translationParams from the loginAlert object to the si-inline-notification component. This aligns with the AlertConfig model and ensures that the notification can display actions and use translation parameters as intended. The changes are straightforward and resolve the described issue effectively.

@github-actions
Copy link

Copy link
Member

@dauriamarco dauriamarco left a comment

Choose a reason for hiding this comment

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

Thank you @ljanner 👌 Could you also add an example for this and maybe an e2e to test it? In the si-landing-page-single-sign-on-login.ts for example:

  ngOnInit(): void {
    // note: this is normally in translation file, this is for demo proposes only
    this.translate.set('DEMO.CONFIRM_LOGIN_MSG', 'Hi {{user}}! You have logged in.');
  }

  ssoLogin(): void {
    this.loginAlert.set({
      severity: 'success',
      message: 'DEMO.CONFIRM_LOGIN_MSG',
      translationParams: { user: 'User' }
    });
  }

@ljanner ljanner force-pushed the fix/landing-page-login-alert branch from b07be02 to 1865997 Compare February 20, 2026 20:26
@ljanner
Copy link
Member Author

ljanner commented Feb 20, 2026

Thank you @ljanner 👌 Could you also add an example for this and maybe an e2e to test it? In the si-landing-page-single-sign-on-login.ts for example: ...

Thanks for the really helpful snippet, added an e2e test 🙏

@ljanner ljanner requested a review from dauriamarco February 20, 2026 20:28
Copy link
Member

@dauriamarco dauriamarco left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@dauriamarco dauriamarco force-pushed the fix/landing-page-login-alert branch from 3683bbe to 1865997 Compare February 20, 2026 20:54
@dauriamarco dauriamarco force-pushed the fix/landing-page-login-alert branch from 1865997 to e360148 Compare February 20, 2026 20:55
@github-actions
Copy link

Code Coverage

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

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Landing page login alert does not use translation params

2 participants