Skip to content

refactor(module:pageheader): insure compatibility RxJs 6 #9052

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

Merged

Conversation

Nicoss54
Copy link
Collaborator

@Nicoss54 Nicoss54 commented Mar 15, 2025

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Application (the showcase website) / infrastructure changes
  • Other... Please describe:

What is the current behavior?

paheHeader use observed getter insteadr observers which is not compatible with RxJs 6 still suppoprt by Angular core

Issue Number: N/A

What is the new behavior?

pagheHeader usethe oberservers property

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@Nicoss54 Nicoss54 requested a review from CK110 as a code owner March 15, 2025 19:16
Copy link

zorro-bot bot commented Mar 15, 2025

This preview will be available after the AzureCI is passed.

Copy link

codecov bot commented Mar 15, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.94%. Comparing base (f0e9d7f) to head (e2588a0).
Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #9052   +/-   ##
=======================================
  Coverage   91.94%   91.94%           
=======================================
  Files         559      559           
  Lines       19773    19773           
  Branches     3050     3050           
=======================================
  Hits        18181    18181           
  Misses       1267     1267           
  Partials      325      325           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Nicoss54 Nicoss54 requested a review from Laffery March 15, 2025 19:44
Copy link
Collaborator

@Laffery Laffery left a comment

Choose a reason for hiding this comment

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

Same to #8454.

This 'observed' property does not yet exist in rxjs6. Since angular/core still supports rxjs6, we can't do this yet.

Further more, Line 134 should use observers instead of observed as well

@Nicoss54 Nicoss54 force-pushed the refactor/deprecated-code-pageheader branch from 6ff9127 to 49dc100 Compare March 15, 2025 23:39
@Nicoss54 Nicoss54 changed the title refactor(module:pageheader): deprecated code observers refactor(module:pageheader): insure compatibility RxJs 6 Mar 15, 2025
@Nicoss54 Nicoss54 requested a review from Laffery March 15, 2025 23:42
@Nicoss54
Copy link
Collaborator Author

Same to #8454.

This 'observed' property does not yet exist in rxjs6. Since angular/core still supports rxjs6, we can't do this yet.

Further more, Line 134 should use observers instead of observed as well

done

@Nicoss54 Nicoss54 self-assigned this Mar 15, 2025
@Nicoss54 Nicoss54 force-pushed the refactor/deprecated-code-pageheader branch from f24f7a3 to e2588a0 Compare March 16, 2025 13:18
@Nicoss54 Nicoss54 requested a review from Laffery March 16, 2025 13:18
Copy link
Collaborator

@Laffery Laffery left a comment

Choose a reason for hiding this comment

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

LGTM

@Laffery Laffery merged commit 815fcfd into NG-ZORRO:master Mar 17, 2025
9 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants