Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(agora): updating PrimeNG, fixing toast (AG-1607) #2970

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

sagely1
Copy link
Contributor

@sagely1 sagely1 commented Jan 21, 2025

Description

This fixes the issue of toasts not displaying. An http interceptor takes errors and displays a PrimeNG global toast for errors. The original design used colors that were not accessible (white on light yellow) so some contrast was added for readability but perhaps colors can be redesigned further.

Original:
image

Modified:
image

This also upgrades PrimeNG to the latest version.

import { withInterceptorsFromDi, provideHttpClient } from '@angular/common/http';
import { provideAnimations } from '@angular/platform-browser/animations';
import { provideHttpClient, withInterceptors } from '@angular/common/http';
import { provideAnimationsAsync } from '@angular/platform-browser/animations/async';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is new for PrimeNG v19

label="{{ type.label }}"
inputId="download-radio"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

syntax change for radio buttons in PrimeNG v19

getInstance() {
return new Rollbar(this.rollbarConfig);
}
export function rollbarFactory() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

newest version of Angular requires a factory method for this to work.

provideHttpClient(withInterceptors([httpErrorInterceptor])),
{
provide: RollbarService,
useFactory: rollbarFactory,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

need factory now in latest Angular

@sagely1 sagely1 marked this pull request as ready for review January 21, 2025 19:42
@sagely1 sagely1 requested review from a team and tschaffter as code owners January 21, 2025 19:42
@sagely1 sagely1 changed the title updating primeng, fixing toast updating primeng, fixing toast (AG-1607) Jan 21, 2025
@sagely1 sagely1 changed the title updating primeng, fixing toast (AG-1607) feat(agora); updating primeng, fixing toast (AG-1607) Jan 21, 2025
@sagely1 sagely1 changed the title feat(agora); updating primeng, fixing toast (AG-1607) feat(agora): updating primeng, fixing toast (AG-1607) Jan 21, 2025
@sagely1 sagely1 changed the title feat(agora): updating primeng, fixing toast (AG-1607) feat(agora): updating PrimeNG, fixing toast (AG-1607) Jan 21, 2025
@sagely1 sagely1 changed the title feat(agora): updating PrimeNG, fixing toast (AG-1607) fix(agora): updating PrimeNG, fixing toast (AG-1607) Jan 21, 2025
@tschaffter
Copy link
Member

@rrchai, could you please review this PR and verify that OC functions correctly after the PrimeNG update? Thank you!

@tschaffter tschaffter requested a review from rrchai January 21, 2025 20:14
apps/agora/app/project.json Show resolved Hide resolved
libs/agora/services/src/lib/http-interceptor.ts Outdated Show resolved Hide resolved
@rrchai
Copy link
Contributor

rrchai commented Jan 22, 2025

could you please review this PR and verify that OC functions correctly after the PrimeNG update?

@tschaffter The OC app would need similar adjustments if primeNG is upgraded to v19. Would you like me to create a separated pr for the changes and merge into this pr?

@tschaffter
Copy link
Member

Would you like me to create a separated pr for the changes and merge into this pr?

@rrchai Yes, thanks!

@tschaffter
Copy link
Member

@sagely1 This PR fails to build the affected project. There is also many warnings about @import being deprecated. Could @use be used instead?

@rrchai Can you provide an ETA for your review and OC patch?

Copy link
Collaborator

@hallieswan hallieswan left a comment

Choose a reason for hiding this comment

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

Approved if all CI checks pass! We should remove ROLLBAR_TOKEN from apps/agora/app/.env.example in a future PR though.

@sagely1
Copy link
Contributor Author

sagely1 commented Jan 23, 2025

@sagely1 This PR fails to build the affected project. There is also many warnings about @import being deprecated. Could @use be used instead?

@rrchai Can you provide an ETA for your review and OC patch?

@import warnings are about syntax SASS will be deprecating. I have created an issue to track: AG-1637

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants