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(module:message): fix the z-index of overlay #8081

Merged
merged 1 commit into from
Sep 18, 2023

Conversation

HyperLife1119
Copy link
Collaborator

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[x] 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?

Issue Number: #8010

image

由于 .cdk-global-overlay-wrapper 是绝对定位,给 .cdk-global-overlay-wrapper 的子元素设置 z-index 是无效的。

What is the new behavior?

image

image

z-index 样式设置到 .cdk-global-overlay-wrapper 上。

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

在此之前的 nz 的 cdk-overlay 预建样式是不完整的,此 PR 将导入完整的 @angular/cdk/overlay-prebuilt.css,用户不再需要自行导入该样式,我希望这点会在 CHANGELOG 中特别指出

@HyperLife1119 HyperLife1119 requested a review from a team as a code owner September 11, 2023 07:04
@zorro-bot
Copy link

zorro-bot bot commented Sep 11, 2023

This preview will be available after the AzureCI is passed.

Comment on lines -40 to -42
&.ant-modal-mask {
opacity: 1;
}
Copy link
Collaborator Author

@HyperLife1119 HyperLife1119 Sep 11, 2023

Choose a reason for hiding this comment

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

这里的 patch 样式是多余的,因为 .cdk-overlay-backdrop 本身会控制自己的 opacity:

image

@codecov
Copy link

codecov bot commented Sep 11, 2023

Codecov Report

Merging #8081 (7dbfe10) into master (7f7c155) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head 7dbfe10 differs from pull request most recent head 55cb870. Consider uploading reports for the commit 55cb870 to get more accurate results

@@           Coverage Diff           @@
##           master    #8081   +/-   ##
=======================================
  Coverage   91.64%   91.65%           
=======================================
  Files         515      515           
  Lines       17640    17640           
  Branches     2790     2790           
=======================================
+ Hits        16167    16168    +1     
+ Misses       1175     1174    -1     
  Partials      298      298           
Files Changed Coverage Δ
components/message/base.ts 93.44% <100.00%> (ø)

... and 1 file with indirect coverage changes

@RickyLin
Copy link

RickyLin commented Oct 8, 2023

Hi @HyperLife1119,
The changes in this PR leaded to the following error during my sample Angular 16.2 project:

./src/theme.less - Error: Module build failed (from ./node_modules/.pnpm/css-loader@6.8.1_webpack@5.88.2/node_modules/css-loader/dist/cjs.js):
Error: Can't resolve '../node_modules/ng-zorro-antd/style/~@angular/cdk/overlay-prebuilt.css' in 'R:\Projects\tmp\testsite\src'

./src/theme.less?ngGlobalStyle - Error: Module build failed (from ./node_modules/.pnpm/mini-css-extract-plugin@2.7.6_webpack@5.88.2/node_modules/mini-css-extract-plugin/dist/loader.js):
HookWebpackError: Module build failed (from ./node_modules/.pnpm/css-loader@6.8.1_webpack@5.88.2/node_modules/css-loader/dist/cjs.js):
Error: Can't resolve '../node_modules/ng-zorro-antd/style/~@angular/cdk/overlay-prebuilt.css' in 'R:\Projects\tmp\testsite\src'

I have a theme.less file under src folder with content below (generated by ng add ng-zorro-antd and choose "Set up custom theme file" to yes)

@import "../node_modules/ng-zorro-antd/ng-zorro-antd.less";

It seems the tilde in this patch.less file can't be recognized.

@import '~@angular/cdk/overlay-prebuilt.css';

Are you able to build successfully in this case?

Thanks.

@HyperLife1119
Copy link
Collaborator Author

I use npm to install dependencies and everything works fine.

@RickyLin
Copy link

RickyLin commented Oct 8, 2023

Interesting, I'm using pnpm. Not sure if that's the reason, I'll try npm when I get a chance.
Thanks.

@RickyLin
Copy link

RickyLin commented Oct 9, 2023

It's definitely an issue of pnpm, I have to ng add @angular/cdk explicitly on my project other than depending on the cdk dependency of ng-zorro-antd itself to make it work.

@OriginRing
Copy link
Collaborator

Hi, @HyperLife1119 Are these issues(#8119 #8106 ) caused by that pr, please take a look? Also the switching theme on the official website is unusually

@HyperLife1119
Copy link
Collaborator Author

@OriginRing 通过 #8122 修复这一问题。

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